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

core/state: fix runaway alloc caused by prefetcher heap escape #30629

Merged
merged 2 commits into from
Oct 20, 2024

Conversation

karalabe
Copy link
Member

This is a trivial change, which seems large-sh, but it's a nobrainer copy-paste fix.

The state prefetched works on both accounts and addresses. The former is 20 bytes the latter 32. To avoid duplicating code, it was implemented by []byte type. Unfortunately, this turned out to be an issue: whereas previously addresses and hashes passed around in statedb were by value on the stack, all of a sudden the addr[:] conversion forces all of them onto the heap, increasing allocations immensely.

This PR "simply" splits the address and hash tracking inside the prefetcher so it can pass around stack values instead of heap items. Fixes the issue, allocation count for my burnt pix benchmarker:

Before:

EVM gas used:    5642735088
execution time:  12.939730959s
allocations:     5372681
allocated bytes: 300059656

After:

EVM gas used:    5642735088
execution time:  12.930701083s
allocations:     915445
allocated bytes: 175248088

@karalabe karalabe added this to the 1.14.12 milestone Oct 18, 2024
@holiman
Copy link
Contributor

holiman commented Oct 19, 2024

LGTM

[user@work go-ethereum]$ ./evm-master run --prestate ./burntpix.json --receiver 0x49206861766520746F6F206D7563682074696D65  --input 0xa4de9ab4000000000000000000000000000000000000000000000000000000000F1FD58E000000000000000000000000000000000000000000000000000000000007A120 --bench | tail -c +131 | sed 's/[0]*$//' | xxd -r -p > output.svg
INFO [10-19|18:52:17.614] Persisted trie from memory database      nodes=10 size=1.20KiB time="62.243µs" gcnodes=0 gcsize=0.00B gctime=0s livenodes=0 livesize=0.00B
EVM gas used:    5642735088
execution time:  45.437703966s
allocations:     5372884
allocated bytes: 300137048
[user@work go-ethereum]$ ./evm-30629 run --prestate ./burntpix.json --receiver 0x49206861766520746F6F206D7563682074696D65  --input 0xa4de9ab4000000000000000000000000000000000000000000000000000000000F1FD58E000000000000000000000000000000000000000000000000000000000007A120 --bench | tail -c +131 | sed 's/[0]*$//' | xxd -r -p > output.svg
INFO [10-19|18:53:14.933] Persisted trie from memory database      nodes=10 size=1.20KiB time="69.123µs" gcnodes=0 gcsize=0.00B gctime=0s livenodes=0 livesize=0.00B
EVM gas used:    5642735088
execution time:  38.598118386s
allocations:     915671
allocated bytes: 175330728

@holiman
Copy link
Contributor

holiman commented Oct 19, 2024

With #30500 on top (my branch journal_on_heapfix)

[user@work go-ethereum]$ ./evm-30629-30500 run --prestate ./burntpix.json --receiver 0x49206861766520746F6F206D7563682074696D65  --input 0xa4de9ab4000000000000000000000000000000000000000000000000000000000F1FD58E000000000000000000000000000000000000000000000000000000000007A120 --bench | tail -c +131 | sed 's/[0]*$//' | xxd -r -p > output.svg
INFO [10-19|19:11:49.588] Persisted trie from memory database      nodes=10 size=1.20KiB time="57.897µs" gcnodes=0 gcsize=0.00B gctime=0s livenodes=0 livesize=0.00B
EVM gas used:    5642735088
execution time:  44.355517077s
allocations:     30219
allocated bytes: 30318416

That's 30M allocated bytes, instead of 175M. The bulk of the burntpix mem is in storage journalling

Screenshot 2024-10-19 at 19-13-51 evm inuse_space

Which goes away with the set-based journalling

Screenshot 2024-10-19 at 19-14-44 evm-30629-30500 inuse_space

@karalabe karalabe merged commit babd5d8 into ethereum:master Oct 20, 2024
3 checks passed
holiman pushed a commit that referenced this pull request Nov 19, 2024
Co-authored-by: lightclient <lightclient@protonmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants