Skip to content

Commit

Permalink
Fix performance regression caused by a bug fix
Browse files Browse the repository at this point in the history
a180597 in erlang#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.
  • Loading branch information
bjorng committed Aug 28, 2024
1 parent b66938c commit bb4edc8
Showing 1 changed file with 42 additions and 8 deletions.
50 changes: 42 additions & 8 deletions erts/emulator/beam/erl_gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
/*
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;

/*
Expand Down

0 comments on commit bb4edc8

Please sign in to comment.