Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix performance regression caused by a bug fix #8751

Merged
merged 1 commit into from
Sep 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions erts/emulator/beam/beam_bp.c
Original file line number Diff line number Diff line change
Expand Up @@ -925,8 +925,6 @@ static void restore_cp_after_trace(Process *c_p, const Eterm cp_save[2]) {
}

static ERTS_INLINE Uint get_allocated_words(Process *c_p, Sint allocated) {
if (c_p->abandoned_heap)
return allocated + c_p->htop - c_p->heap + c_p->mbuf_sz;
return allocated + c_p->htop - c_p->high_water + c_p->mbuf_sz;
}

Expand Down
6 changes: 2 additions & 4 deletions erts/emulator/beam/erl_bif_trace.c
Original file line number Diff line number Diff line change
Expand Up @@ -2899,12 +2899,10 @@ new_seq_trace_token(Process* p, int ensure_new_heap)
make_small(p->seq_trace_lastcnt));
}
else if (ensure_new_heap) {
Eterm *mature = p->abandoned_heap ? p->abandoned_heap : p->heap;
Uint mature_size = p->high_water - mature;
Eterm* tpl = tuple_val(SEQ_TRACE_TOKEN(p));
ASSERT(arityval(tpl[0]) == 5);
if (ErtsInBetween(tpl, OLD_HEAP(p), OLD_HEND(p)) ||
ErtsInArea(tpl, mature, mature_size*sizeof(Eterm))) {

if (!ErtsInBetween(tpl, p->high_water, p->hend)) {
hp = HAlloc(p, 6);
sys_memcpy(hp, tpl, 6*sizeof(Eterm));
SEQ_TRACE_TOKEN(p) = make_tuple(hp);
Expand Down
111 changes: 81 additions & 30 deletions erts/emulator/beam/erl_gc.c
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ do { \
ASSERT((p)->abandoned_heap || (P)->heap_sz == (P)->hend - (P)->heap); \
ASSERT((P)->heap <= (P)->htop && (P)->htop <= (P)->hend); \
ASSERT((P)->heap <= (P)->stop && (P)->stop <= (P)->hend); \
ASSERT((p)->abandoned_heap || ((P)->heap <= (P)->high_water && (P)->high_water <= (P)->hend)); \
ASSERT(((P)->heap <= (P)->high_water && (P)->high_water <= (P)->hend)); \
OverRunCheck((P)); \
} while (0)
#else
Expand Down Expand Up @@ -525,12 +525,28 @@ 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);
/* Allocate one extra word at the end to save the high water mark. */
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 All @@ -543,7 +559,7 @@ delay_garbage_collection(Process *p, int need, int fcalls)
Uint used = orig_htop - orig_heap;
hfrag->used_size = used;
p->mbuf_sz += used;
ASSERT(hfrag->used_size <= hfrag->alloc_size);
ASSERT(hfrag->used_size <= hfrag->alloc_size-1);
ASSERT(!hfrag->off_heap.first && !hfrag->off_heap.overhead);
hfrag->next = p->mbuf;
p->mbuf = hfrag;
Expand All @@ -559,11 +575,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 @@ -637,21 +648,39 @@ young_gen_usage(Process *p, Uint *ext_msg_usage)
return hsz;
}

#define ERTS_GET_ORIG_HEAP(Proc, Heap, HTop) \
do { \
Eterm *aheap__ = (Proc)->abandoned_heap; \
if (!aheap__) { \
(Heap) = (Proc)->heap; \
(HTop) = (Proc)->htop; \
} \
else { \
(Heap) = aheap__; \
if ((Proc)->flags & F_ABANDONED_HEAP_USE) \
(HTop) = aheap__ + aheap__[(Proc)->heap_sz-1]; \
else \
(HTop) = aheap__ + (Proc)->heap_sz; \
} \
} while (0)
static Eterm*
get_orig_heap(Process *p, Eterm **p_htop, Eterm **p_high_water) {
Eterm *aheap = p->abandoned_heap;
Eterm *htop;

/* See delay_garbage_collection(). */

ASSERT(aheap != NULL);

if (p->flags & F_ABANDONED_HEAP_USE) {
htop = aheap + aheap[p->heap_sz-1];
} else {
htop = aheap + p->heap_sz;
}

*p_htop = htop;

if (p_high_water) {
Eterm *high_water;

high_water = (Eterm *)(p->hend[0]);

ASSERT(aheap <= high_water);
ASSERT(high_water <= htop);

/* The high water pointer must be aligned to a word boundary. */
ASSERT(((UWord) high_water) % sizeof(UWord) == 0);

*p_high_water = high_water;
}

return aheap;
}

static ERTS_INLINE void
check_for_possibly_long_gc(Process *p, Uint ygen_usage)
Expand Down Expand Up @@ -1383,12 +1412,32 @@ 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. */
ASSERT(((UWord) high_water) % sizeof(UWord) == 0);
}
#endif

mature_size = high_water - mature;

need += S_RESERVED;

/*
Expand Down Expand Up @@ -3659,9 +3708,11 @@ erts_process_gc_info(Process *p, Uint *sizep, Eterm **hpp,
ERTS_CT_ASSERT(sizeof(values)/sizeof(*values) == ERTS_PROCESS_GC_INFO_MAX_TERMS);

if (p->abandoned_heap) {
Eterm *htop, *heap;
ERTS_GET_ORIG_HEAP(p, heap, htop);
values[3] = HIGH_WATER(p) - heap;
Eterm *htop, *heap, *high_water;

heap = get_orig_heap(p, &htop, &high_water);

values[3] = high_water - heap;
values[6] = htop - heap;
}

Expand Down Expand Up @@ -3906,7 +3957,7 @@ erts_dbg_within_proc(Eterm *ptr, Process *p, Eterm *real_htop)
Eterm *htop, *heap;

if (p->abandoned_heap) {
ERTS_GET_ORIG_HEAP(p, heap, htop);
heap = get_orig_heap(p, &htop, NULL);
if (heap <= ptr && ptr < htop)
return 1;
}
Expand Down Expand Up @@ -4017,7 +4068,7 @@ erts_dbg_check_heap_terms(int (*check_eterm)(Eterm),
Eterm *htop, *heap;

if (p->abandoned_heap) {
ERTS_GET_ORIG_HEAP(p, heap, htop);
heap = get_orig_heap(p, &htop, NULL);
if (!check_all_heap_terms_in_range(check_eterm,
heap, htop))
return 0;
Expand Down
8 changes: 8 additions & 0 deletions erts/emulator/beam/erl_process.h
Original file line number Diff line number Diff line change
Expand Up @@ -1057,7 +1057,15 @@ struct process {

Eterm* heap; /* Heap start */
Eterm* hend; /* Heap end */

/* If abandoned_heap is not a NULL pointer, it points to the heap
* that was active when delay_garbage_collection() in erl_gc.c was
* called. The high water mark that was active at that time is
* saved in p->hend[0].
*/

Eterm* abandoned_heap;

Uint heap_sz; /* Size of heap in words */
Uint min_heap_size; /* Minimum size of heap (in words). */
Uint min_vheap_size; /* Minimum size of virtual heap (in words). */
Expand Down