diff options
author | Xavier Leroy <xavierleroy@users.noreply.github.com> | 2021-01-07 14:43:42 +0100 |
---|---|---|
committer | Xavier Leroy <xavier.leroy@college-de-france.fr> | 2021-01-07 14:47:35 +0100 |
commit | 2f0f2f1b8b5385cd885d44cb8f4705124ff8d34f (patch) | |
tree | fd7499c0a24c96418956e5b752fa5e47a0700498 | |
parent | e322556b0a9097a2eff2117476193b773e1b947f (diff) | |
download | ocaml-2f0f2f1b8b5385cd885d44cb8f4705124ff8d34f.tar.gz |
Gc.set incorrectly handles the three `custom_*` fields (#10125)
Some `Long_val` conversions were missing.
This was setting the wrong values for the `custom_` parameters,
causing the major GC to work too much, slowing down some programs.
Add regression test.
Fixes: #9326
(cherry picked from commit 78321e6ac59e18a54cd9209aa57012f8b47908c9)
-rw-r--r-- | Changes | 7 | ||||
-rw-r--r-- | runtime/gc_ctrl.c | 6 | ||||
-rw-r--r-- | testsuite/tests/regression/pr9326/gc_set.ml | 35 |
3 files changed, 45 insertions, 3 deletions
@@ -1,11 +1,18 @@ OCaml 4.08 maintenance branch: ------------------------------ +### Bug fixes: + - #8855, #8858: Links for tools not created when installing with --disable-installing-byecode-programs (e.g. ocamldep.opt installed, but ocamldep link not created) (David Allsopp, report by Thomas Leonard) +- #9326, #10125: Gc.set incorrectly handles the three `custom_*` fields, + causing a performance regression + (report by Emilio Jesús Gallego Arias, analysis and fix by Stephen Dolan, + code by Xavier Leroy, review by Hugo Heuzard and Gabriel Scherer) + - #9383: Don't assume that AWKPATH includes . (David Allsopp, report by Ian Zimmerman) diff --git a/runtime/gc_ctrl.c b/runtime/gc_ctrl.c index 4b2efa1ac6..0670ed4ef9 100644 --- a/runtime/gc_ctrl.c +++ b/runtime/gc_ctrl.c @@ -475,21 +475,21 @@ CAMLprim value caml_gc_set(value v) /* These fields were added in 4.08.0. */ if (Wosize_val (v) >= 11){ - new_custom_maj = norm_custom_maj (Field (v, 8)); + new_custom_maj = norm_custom_maj (Long_val (Field (v, 8))); if (new_custom_maj != caml_custom_major_ratio){ caml_custom_major_ratio = new_custom_maj; caml_gc_message (0x20, "New custom major ratio: %" ARCH_INTNAT_PRINTF_FORMAT "u%%\n", caml_custom_major_ratio); } - new_custom_min = norm_custom_min (Field (v, 9)); + new_custom_min = norm_custom_min (Long_val (Field (v, 9))); if (new_custom_min != caml_custom_minor_ratio){ caml_custom_minor_ratio = new_custom_min; caml_gc_message (0x20, "New custom minor ratio: %" ARCH_INTNAT_PRINTF_FORMAT "u%%\n", caml_custom_minor_ratio); } - new_custom_sz = Field (v, 10); + new_custom_sz = Long_val (Field (v, 10)); if (new_custom_sz != caml_custom_minor_max_bsz){ caml_custom_minor_max_bsz = new_custom_sz; caml_gc_message (0x20, "New custom minor size limit: %" diff --git a/testsuite/tests/regression/pr9326/gc_set.ml b/testsuite/tests/regression/pr9326/gc_set.ml new file mode 100644 index 0000000000..e9d7dbcd4c --- /dev/null +++ b/testsuite/tests/regression/pr9326/gc_set.ml @@ -0,0 +1,35 @@ +(* TEST +*) + +open Gc + +let min_heap_sz = 524288 (* 512k *) +let maj_heap_inc = 4194304 (* 4M *) + +let _ = + let g1 = Gc.get() in + (* Do not use { g1 with ... }, so that the code will break if more fields + are added to the Gc.control record type *) + Gc.set { minor_heap_size = min_heap_sz; + major_heap_increment = maj_heap_inc; + space_overhead = g1.space_overhead; + verbose = g1.verbose; + max_overhead = g1.max_overhead; + stack_limit = g1.stack_limit; + allocation_policy = g1.allocation_policy; + window_size = g1.window_size; + custom_major_ratio = g1.custom_major_ratio; + custom_minor_ratio = g1.custom_minor_ratio; + custom_minor_max_size = g1.custom_minor_max_size }; + let g2 = Gc.get() in + assert (g2.minor_heap_size = min_heap_sz); + assert (g2.major_heap_increment = maj_heap_inc); + assert (g2.space_overhead = g1.space_overhead); + assert (g2.verbose = g1.verbose); + assert (g2.max_overhead = g1.max_overhead); + assert (g2.stack_limit = g1.stack_limit); + assert (g2.allocation_policy = g1.allocation_policy); + assert (g2.window_size = g1.window_size); + assert (g2.custom_major_ratio = g1.custom_major_ratio); + assert (g2.custom_minor_ratio = g1.custom_minor_ratio); + assert (g2.custom_minor_max_size = g1.custom_minor_max_size) |