Fix performance regression caused by a bug fix #8751
Conversation
bjorng
added
team:VM
CT Test Results 3 files 143 suites 49m 25s ⏱️ For more details on these failures, see this check. Results for commit 9712516. ♻️ This comment has been updated with latest results. To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass. See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally. Artifacts// Erlang/OTP Github Action Bot |
bjorng
force-pushed
the
bjorn/erts/fix-delay-gc/OTP-12909
branch
2 times, most recently
from
e7e458a
to
13df903
Compare
There was a problem hiding this comment.
I don't like the high_water = -1 hack.
I don't like different semantics for high_water on jit vs emu.
What about letting delay_garbage_collection() always set high_water correctly for the active heap (p->high_water = p->heap I suppose).
To save space you could save abandoned_high_water at the end of the heap fragment for the new active heap for example.
|
@sverker I've rewritten the PR as you suggested. |
There was a problem hiding this comment.
I did git grep -iw high_water in erts/emulator and found some places that need tweaks:
beam_bp.c:
get_allocated_words() can be simplified to ignore abandoned_heap.
erl_bif_trace.c:
new_seq_trace_token() must be fixed
erl_gc.c:
ErtsGcQuickSanityCheck() can always assert high_water between heap and hend.
erts_process_gc_info() must be fixed.
bjorng
force-pushed
the
bjorn/erts/fix-delay-gc/OTP-12909
branch
2 times, most recently
from
c5d85e0
to
9712516
Compare
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.
bjorng
force-pushed
the
bjorn/erts/fix-delay-gc/OTP-12909
branch
from
9712516
to
8f811c9
Compare
a180597 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/1andiolist_to_binary/1, may temporarily disable GC, this could lead to noticeable performance regressions.This pull request ensures that it is again possible to do a minor collection when GC is being enabled.