From a52f49bd2c18b548d7d420f4f628b0acda5b62b9 Mon Sep 17 00:00:00 2001 From: Xavier Leroy Date: Thu, 7 Jan 2021 14:43:42 +0100 Subject: 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) --- Changes | 10 +++++++++ runtime/gc_ctrl.c | 6 ++--- testsuite/tests/regression/pr9326/gc_set.ml | 35 +++++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 3 deletions(-) create mode 100644 testsuite/tests/regression/pr9326/gc_set.ml diff --git a/Changes b/Changes index c79f752c12..229f016eb4 100644 --- a/Changes +++ b/Changes @@ -906,6 +906,16 @@ OCaml 4.09.0 (19 September 2019): - #8944: Fix "open struct .. end" on clambda backend (Thomas Refis, review by Leo White, report by Damon Wang and Mark Shinwell) +OCaml 4.08 maintenance branch +----------------------------- + +### Bug fixes: + +- #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) + OCaml 4.08.1 (5 August 2019) ---------------------------- diff --git a/runtime/gc_ctrl.c b/runtime/gc_ctrl.c index e444b9c5cd..f0d0be2b7d 100644 --- a/runtime/gc_ctrl.c +++ b/runtime/gc_ctrl.c @@ -470,21 +470,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) -- cgit v1.2.1