Refactor update_major_slice_work and major GC work units #3628
Conversation
- Rearrange code in this function to separate computation of allocations done
from computation of next slice work budget, to more easily replace the latter
- Better log messages from slice computation
- Separate units of marking and sweeping work, and add unit conversions
- Rename 'work_counter' and 'alloc_counter'
There was a problem hiding this comment.
LGTM. Good to separate the notions of sweep work and mark work (hmm, could use the type system for that). A couple of suggestions, mostly naming. As anticipated, this will mean I have to rework #3587 when rebasing, but of course I'm not going to do that until after your next PR.
| only used locally during marking */ | ||
| static intnat Markwork_sweepwork(intnat sweep_work) | ||
| { | ||
| return sweep_work; |
There was a problem hiding this comment.
Add a comment to each of these functions (or extend the banner comment above) explaining why they are currently nops.
| `work_counter` increases when the GC has done some work. | ||
| do in order to keep up with allocation. Both are in sweep work units. | ||
| `total_work_incurred` increases when we allocate: the number of words | ||
| allocated is converted to GC work units and added to this counter. |
| dom_st->slice_budget); | ||
| return min2(budget, Chunk_size); | ||
| } | ||
|
|
||
| /* Register the work done by a chunk of slice. | ||
| Clear requested_global_major_slice if the work counter has caught up with | ||
| the slice's target counter. */ | ||
| static void commit_major_slice_work(intnat words_done) { | ||
| static void commit_major_slice_sweepwork(intnat words_done) { |
There was a problem hiding this comment.
I think perhaps record_major_slice_sweeping? (or record_sweepwork?) Ditto ditto marking.
| /* We've done enough work by ourselves, no need to interrupt the other | ||
| domains. */ | ||
| dom_st->requested_global_major_slice = 0; | ||
| } | ||
| } | ||
|
|
||
| static intnat get_major_slice_markwork(collection_slice_mode mode) |
| intnat left = ephe_mark(budget, saved_ephe_cycle, EPHE_MARK_DEFAULT); | ||
| intnat work_done = budget - left; | ||
| /* caml_darken is called by ephe_mark, so count the work it does */ | ||
| work_done += mark_work_done_between_slices(); | ||
| ephe_mark_work += work_done; | ||
| commit_major_slice_work (work_done); | ||
| commit_major_slice_markwork(work_done); |
There was a problem hiding this comment.
I'm not sure whether and to what extent ephemeron marking should count as marking (or ephemeron sweeping should count as sweeping). For instance, consider that ephemerons themselves are freed during ephemeron marking. I don't mind much that we account for it in this way, because the numbers are only going to be significant in really weird programs which are probably paced weirdly anyway, but I think we should probably have a comment at least.
| uintnat my_alloc_direct_count = dom_st->allocated_words_direct; | ||
| uintnat my_dependent_count = Wsize_bsize (dom_st->allocated_dependent_bytes); | ||
| dom_st->stat_major_words += dom_st->allocated_words; | ||
| dom_st->stat_major_dependent_bytes += dom_st->allocated_dependent_bytes; |
There was a problem hiding this comment.
Well caught: we were only doing this in caml_finish_marking before.
| intnat alloc_work, extra_work; | ||
| double my_extra_count; | ||
| uintnat heap_sweep_words, total_cycle_work; |
There was a problem hiding this comment.
All these declarations could be at first assignment (but probably not worth touching this code which is about to evaporate).
| @@ -658,134 +702,99 @@ static void update_major_slice_work(intnat howmuch, | |||
| marking phase) before it starts to deallocate dead blocks | |||
| allocated during the previous cycle. [heap_size / 150] is really | |||
| [heap_size * (2/3) / 100] (but faster). */ | |||
| double max_major = | |||
| double custom_max_major = | |||
| caml_heap_size(Caml_state->shared_heap) / 150 * caml_custom_major_ratio; | |||
There was a problem hiding this comment.
caml_heap_size(Caml_state->shared_heap) we already have in heap_words (subject to words/bytes).
Rearrange code in this function to separate computation of allocations done from computation of next slice work budget, to more easily replace the latter
Better log messages from slice computation
Separate units of marking and sweeping work, and add unit conversions
Rename 'work_counter' and 'alloc_counter'