From bb4edc8a6fa612d67bd67af5918baf269acf1e06 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 | 50 +++++++++++++++++++++++++++++++------ 1 file changed, 42 insertions(+), 8 deletions(-) diff --git a/erts/emulator/beam/erl_gc.c b/erts/emulator/beam/erl_gc.c index c821ff9f3a42..ce57697db103 100644 --- a/erts/emulator/beam/erl_gc.c +++ b/erts/emulator/beam/erl_gc.c @@ -525,12 +525,27 @@ delay_garbage_collection(Process *p, int need, int fcalls) ssz = orig_hend - orig_stop; hsz = ssz + need + ERTS_DELAY_GC_EXTRA_FREE + S_RESERVED; - hfrag = new_message_buffer(hsz); + hfrag = new_message_buffer(hsz + 1); copy_erlang_stack(p, &hfrag->mem[0], hsz); p->heap = p->htop = &hfrag->mem[0]; - p->hend = hend = &hfrag->mem[hsz]; + hend = &hfrag->mem[hsz]; + + /* Save the original high water mark at the end of the current + * heap to make it possible to do a minor GC later. */ + if (p->abandoned_heap) { + *hend = (Eterm) (p->hend[0]); + } else { + *hend = (Eterm) p->high_water; + } + + p->hend = hend; + + /* Keep the high water mark pointing into the current heap to ensure + * that the test for the safe range in the update_record_in_place (JIT) + * stays honest. */ + p->high_water = p->heap; if (p->abandoned_heap) { /* @@ -559,11 +574,6 @@ delay_garbage_collection(Process *p, int need, int fcalls) } p->abandoned_heap = orig_heap; erts_adjust_memory_break(p, orig_htop - p->high_water); - - /* 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. */ - p->high_water = (Eterm *) (Uint) -1; } #ifdef CHECK_FOR_HOLES @@ -1383,12 +1393,36 @@ minor_collection(Process* p, ErlHeapFragment *live_hf_end, Uint ygen_usage, Uint *recl) { Eterm *mature = p->abandoned_heap ? p->abandoned_heap : p->heap; - Uint mature_size = p->high_water - mature; + Eterm *high_water; + Uint mature_size; Uint size_before = ygen_usage; #ifdef DEBUG Uint debug_tmp = 0; #endif + if (p->abandoned_heap) { + /* See delay_garbage_collection(). */ + high_water = (Eterm *)(p->hend[0]); + } else { + high_water = p->high_water; + } + +#ifdef DEBUG + if (p->abandoned_heap) { + ASSERT(p->abandoned_heap <= high_water); + ASSERT(high_water - p->abandoned_heap <= size_before); + + /* The high water pointer must be aligned to a word boundary. */ +#ifdef ARCH_64 + ASSERT((((Uint) high_water) & 0x07) == 0); +#else + ASSERT((((Uint) high_water) & 0x03) == 0); +#endif + } +#endif + + mature_size = high_water - mature; + need += S_RESERVED; /*