Separate the mechanisms and APIs for dependent memory and custom blocks #3597
Conversation
Previously, allocating a block with caml_alloc_custom or caml_alloc_custom_mem would both register a custom block finaliser and accelerate the GC. Now, caml_alloc_custom(_mem) has no effect on GC, and the functions caml_adjust_gc_speed and caml_adjust_minor_gc_speed become noops. The GC speed increase for dependent memory happens only through caml_alloc_dependent_memory and caml_free_dependent_memory. The function caml_alloc_custom_dep is available to call both caml_alloc_custom and caml_alloc_dependent_memory. However, the user must ensure to call caml_free_dependent_memory in their finaliser. (Bigarrays have already been updated to use this API in a previous patch) This is to pave the way for a new GC pacing policy. However, no change to pacing is made by this patch - the GC should perform as previously on e.g. bigarray values, which have been ported to use the new API. (Most of the code in this patch was written by @damiendoligez)
| static value alloc_custom_gen (const struct custom_operations * ops, | ||
| uintnat bsz, | ||
| mlsize_t mem, | ||
| mlsize_t max_major, | ||
| mlsize_t max_minor, | ||
| int minor_ok, |
There was a problem hiding this comment.
Any chance of making these bool while we're here?
| CAMLexport value caml_alloc_custom(const struct custom_operations * ops, | ||
| uintnat bsz, | ||
| mlsize_t mem, | ||
| mlsize_t max) | ||
| { | ||
| return caml_alloc_custom0(ops, bsz, mem, max, 0); | ||
| return alloc_custom_gen(ops, bsz, /* minor_ok: */ 1, /* local: */ 0); |
There was a problem hiding this comment.
Should this call caml_memprof_sample_block?
There was a problem hiding this comment.
Ah, no. The proxy block is already sampled, and this is now the path for custom blocks without foreign memory.
|
|
||
| DOMAIN_STATE(uintnat, dependent_size) | ||
| DOMAIN_STATE(uintnat, dependent_allocated) | ||
| /* How much external memory is currenty held by the minor and major heap. */ |
| intnat alloc_work, dependent_work, extra_work, new_work; | ||
| intnat my_alloc_count, my_alloc_direct_count, my_dependent_count; | ||
| intnat alloc_work, extra_work, new_work; | ||
| intnat my_alloc_count, my_alloc_direct_count; |
There was a problem hiding this comment.
I'm only skimming this function as I review this PR because I know it's about to change radically (in the next week or so).
| / 100 * caml_custom_minor_ratio){ | ||
| caml_request_minor_gc (); | ||
| } | ||
| }else{ |
There was a problem hiding this comment.
horrible whitespace in this function.
| }else{ | ||
| caml_add_dependent_bytes (Caml_state->shared_heap, nbytes); | ||
| Caml_state->allocated_dependent_bytes += nbytes; | ||
| /* FIXME sdolan: what's the right condition here? */ |
There was a problem hiding this comment.
An excellent question. I don't really believe in any of this caml_custom_get_max_major() stuff. "2/3 of 44% of the heap size, divided by 5". I keep expecting to subtract the number I first thought of and tada: it's the playing card hidden in the orange.
Previously, allocating a block with caml_alloc_custom or caml_alloc_custom_mem would both register a custom block finaliser and accelerate the GC.
Now, caml_alloc_custom(_mem) has no effect on GC, and the functions caml_adjust_gc_speed and caml_adjust_minor_gc_speed become noops. The GC speed increase for dependent memory happens only through caml_alloc_dependent_memory and caml_free_dependent_memory.
The function caml_alloc_custom_dep is available to call both caml_alloc_custom and caml_alloc_dependent_memory. However, the user must ensure to call caml_free_dependent_memory in their finaliser. (Bigarrays have already been updated to use this API in a previous patch)
This is to pave the way for a new GC pacing policy. However, no change to pacing is made by this patch - the GC should perform as previously on e.g. bigarray values, which have been ported to use the new API.
(Most of the code in this patch was written by @damiendoligez )