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 27, 2024
1 parent ee9628e commit 13df903
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 3 deletions.
31 changes: 28 additions & 3 deletions erts/emulator/beam/erl_gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1382,15 +1387,35 @@ 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;
#endif

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 != (Eterm *) (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
*/
Expand Down
5 changes: 5 additions & 0 deletions erts/emulator/beam/erl_process.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 13df903

Please sign in to comment.