From e7e458acb7915c239d8c9fc40b102c67dabea2f8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bj=C3=B6rn=20Gustavsson?= Date: Tue, 27 Aug 2024 08:35:27 +0200 Subject: [PATCH] Fix performance regression caused by a bug fix a1805974f864fe in #8539, a correction for an unsafe in-place update of a record, had the side effect of causing a major (fullsweep) garbage collection when GC has been temporarily disabled and then again enabled. Since many BIFs, such as `term_to_binary/1` and `iolist_to_binary/1`, may temporarily disable GC, this could lead to noticeable performance regressions. This commit ensures that it is again possible to do a minor collection when GC is being enabled. --- erts/emulator/beam/erl_gc.c | 31 ++++++++++++++++++++++++++++--- erts/emulator/beam/erl_process.h | 5 +++++ 2 files changed, 33 insertions(+), 3 deletions(-) diff --git a/erts/emulator/beam/erl_gc.c b/erts/emulator/beam/erl_gc.c index c821ff9f3a42..fb044e2484cb 100644 --- a/erts/emulator/beam/erl_gc.c +++ b/erts/emulator/beam/erl_gc.c @@ -560,10 +560,15 @@ delay_garbage_collection(Process *p, int need, int fcalls) p->abandoned_heap = orig_heap; erts_adjust_memory_break(p, orig_htop - p->high_water); +#ifdef BEAMASM /* Point at the end of the address range to ensure that * test for the safe range in the new heap in the - * update_record_in_place instruction fails. */ + * update_record_in_place instruction fails. Save the previous + * value of the high water mark to make it possible to make the + * next GC a minor one. */ + p->abandoned_high_water = p->high_water; p->high_water = (Eterm *) (Uint) -1; +#endif } #ifdef CHECK_FOR_HOLES @@ -1382,8 +1387,8 @@ minor_collection(Process* p, ErlHeapFragment *live_hf_end, Uint need, Eterm* objv, int nobj, Uint ygen_usage, Uint *recl) { - Eterm *mature = p->abandoned_heap ? p->abandoned_heap : p->heap; - Uint mature_size = p->high_water - mature; + Eterm *mature; + Uint mature_size; Uint size_before = ygen_usage; #ifdef DEBUG Uint debug_tmp = 0; @@ -1391,6 +1396,26 @@ minor_collection(Process* p, ErlHeapFragment *live_hf_end, need += S_RESERVED; +#if defined(DEBUG) && defined(BEAMASM) + if (p->abandoned_heap) { + ASSERT(p->abandoned_heap <= p->abandoned_high_water); + ASSERT(p->abandoned_high_water != (Uint) -1); + } +#endif + + if (p->abandoned_heap) { +#ifdef BEAMASM + Eterm *high_water = p->abandoned_high_water; +#else + Eterm *high_water = p->high_water; +#endif + mature = p->abandoned_heap; + mature_size = high_water - mature; + } else { + mature = p->heap; + mature_size = p->high_water - mature; + } + /* * Check if we have gone past the max heap size limit */ diff --git a/erts/emulator/beam/erl_process.h b/erts/emulator/beam/erl_process.h index b3eae36e7089..8e2f558bd389 100644 --- a/erts/emulator/beam/erl_process.h +++ b/erts/emulator/beam/erl_process.h @@ -1113,6 +1113,11 @@ struct process { Uint16 gen_gcs; /* Number of (minor) generational GCs. */ Uint16 max_gen_gcs; /* Max minor gen GCs before fullsweep. */ Eterm *high_water; + +#ifdef BEAMASM + Eterm *abandoned_high_water; +#endif + Eterm *old_hend; /* Heap pointers for generational GC. */ Eterm *old_htop; Eterm *old_heap;