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

Further optimize off-heap traversal during minor GC #5195

Merged
merged 2 commits into from
Sep 30, 2021

Conversation

sverker
Copy link
Contributor

@sverker sverker commented Sep 12, 2021

Only traverse first part of off-heap list that reside on the new-heap, except for writable binaries that are now kept in a separate list.

Fixes #4424.

Earlier during minor GC, traversing the off-heap list was continued to the end over terms residing on the old-heap. This for two reasons:

  1. The BIN_OLD_VHEAP value was calculated by adding the sizes of all ProcBin's on the old-heap.
  2. All writable binaries are handled by shrinking (reallocating) binaries that have not been written to since last GC. Even binaries residing on the old-heap.

To optimize and only traverse the first part the off-heap list that reside on the new-heap, we now

  1. maintain a correct value of BIN_OLD_VHEAP so it does not have to be recalculated during GC.
  2. keep all writable binaries in a separate list Process.wrt_bins that is traversed to the end during every GC.

The aim of this PR has been to maintain the overall GC semantics and especially not alter the value of BIN_OLD_VHEAP or how binaries have been reallocated at the end of a GC.

PB_ACTIVE_WRITER was set without PB_IS_WRITABLE
@sverker sverker added team:VM Assigned to OTP team VM enhancement labels Sep 12, 2021
@sverker sverker self-assigned this Sep 12, 2021
@sverker sverker added the testing currently being tested, tag is used by OTP internal CI label Sep 13, 2021
@sverker sverker requested a review from bjorng September 29, 2021 10:46
Copy link
Contributor

@bjorng bjorng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

erts/emulator/beam/erl_bits.c Outdated Show resolved Hide resolved
erts/emulator/beam/erl_bits.c Show resolved Hide resolved
erts/emulator/beam/erl_gc.c Outdated Show resolved Hide resolved
erts/emulator/beam/erl_gc.c Outdated Show resolved Hide resolved
erts/emulator/beam/erl_gc.c Outdated Show resolved Hide resolved
erts/emulator/beam/erl_gc.c Outdated Show resolved Hide resolved
erts/emulator/beam/erl_gc.c Outdated Show resolved Hide resolved
erts/emulator/beam/erl_gc.c Show resolved Hide resolved
Only traverse first part of off-heap list that reside on the new-heap,
except for writable binaries that are now kept in a separate list.

Earlier during minor GC, traversing the off-heap list was continued
to the end over terms residing on the old-heap. This for two reasons:

1. The BIN_OLD_VHEAP value was calculated by adding the sizes
   of all ProcBin's on the old-heap.
2. All writable binaries are handled by shrinking (reallocating)
   binaries that have not been written to since last GC. Even binaries
   residing on the old-heap.

To optimize and only traverse the first part the off-heap list
that reside on the new-heap, we now

1. maintain a correct value of BIN_OLD_VHEAP so it does not have to be
   recalculated during GC.
2. keep all writable binaries in a separate list Process.wrt_bins
   that is traversed to the end during every GC.

The aim of this commit has been to maintain the overall GC semantics
and especially not alter the value of BIN_OLD_VHEAP or how binaries
have been reallocated at the end of a GC.
@sverker sverker force-pushed the sverker/erts/writable-bin-list branch from 88f15eb to e16e545 Compare September 30, 2021 16:32
@sverker sverker merged commit 0410f98 into erlang:master Sep 30, 2021
@sverker sverker added this to the OTP-25.0 milestone Sep 30, 2021
garazdawi added a commit to garazdawi/otp that referenced this pull request Feb 7, 2022
When creating a new proc bin because tracing has removed the
writable flag from the proc bin, we must make sure to also
create a new sub binary if the sub binary is on the mature
or old heap. When this is not done, the subbinary can be
promoted to the old heap without the proc bin also being there.

Bug was introduced in erlang#5195, aka e16e545
garazdawi added a commit to garazdawi/otp that referenced this pull request Feb 7, 2022
When creating a new proc bin because tracing has removed the
writable flag from the proc bin, we must make sure to also
create a new sub binary if the sub binary is on the mature
or old heap. When this is not done, the subbinary can be
promoted to the old heap without the proc bin also being there.

Bug was introduced in erlang#5195, aka e16e545
garazdawi added a commit to garazdawi/otp that referenced this pull request Feb 11, 2022
When creating a new proc bin because tracing has removed the
writable flag from the proc bin, we must make sure to also
create a new sub binary if the sub binary is on the mature
or old heap. When this is not done, the subbinary can be
promoted to the old heap without the proc bin also being there.

Bug was introduced in erlang#5195, aka e16e545
@sverker sverker deleted the sverker/erts/writable-bin-list branch June 16, 2022 10:02
max-au added a commit to max-au/otp that referenced this pull request Dec 18, 2022
When writable binaries were introduced in erlang#5195, they weren't
added to the output of `process_info(Pid, binary)`. It led to
interesting effects of binaries suddenly disappearing after
running implicit GC caused by BIF calls, including `process_info`
itself. Effectively, running `process_info(self(), binary)` twice
in a row resulted in different lists, because first call triggers
GC clearing out original binary from the off-heap list:

    start() ->
        OffHeapBin = crypto:strong_rand_bytes(1024 * 1024),
        Bin = <<OffHeapBin/binary, "a">>,
        Before = erlang:process_info(self(), [binary]),
        After = erlang:process_info(self(), [binary]),
        Before =/= After andalso erlang:display({wtf, Before, After}),
        Bin.

Fixes erlang#6573
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement team:VM Assigned to OTP team VM testing currently being tested, tag is used by OTP internal CI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ERL-1347: Performance issue with file:read_line and ref binaries
2 participants