Fix minor-heap allocation computation in Gc.counters() #12784
Conversation
There was a problem hiding this comment.
It appears to have been a micro-slip when 4.06.0 was merged into multicore - ocaml-multicore/ocaml-multicore@e8c6908
runtime/gc_ctrl.c
Outdated
| @@ -111,7 +111,7 @@ CAMLprim value caml_gc_counters(value v) | |||
| /* get a copy of these before allocating anything... */ | |||
| double minwords = Caml_state->stat_minor_words | |||
| + ((double) Wsize_bsize ((uintnat)Caml_state->young_end - | |||
| (uintnat) Caml_state->young_ptr)) / sizeof(value); | |||
| (uintnat) Caml_state->young_ptr)); | |||
There was a problem hiding this comment.
This module currently has inconsistent code, with Wsize_bsize used here and / sizeof(value) used above in caml_gc_minor_words_unboxed. Could you solve this by calling caml_gc_minor_words_unboxed here?
There was a problem hiding this comment.
I apologise for so much discussion over one line, but in the commit I referenced above, the / sizeof(value) was added in both places (erroneously in this second one), where I think the fix really should have been to use Wsize_bsize in caml_gc_minor_words_unboxed rather than / sizeof(value) 🙈
There was a problem hiding this comment.
Arguably, since both young_ptr and young_end are value *, one could just do the subtraction, without any casts, divisions, or macro applications....
NickBarnes
force-pushed
the
nick-gc-counters
branch
2 times, most recently
from
81e84f5
to
c6c67b2
Compare
|
This also has a user-visible fix to |
NickBarnes
force-pushed
the
nick-gc-counters
branch
from
c6c67b2
to
0759be0
Compare
NickBarnes
force-pushed
the
nick-gc-counters
branch
from
0759be0
to
c3a48c3
Compare
Gc.counters()computes minor-heap allocation incorrectly: it divides the allocation in words since the last collection by the word size.