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

Reduce Evm Stackspace clearing #5826

Merged
merged 32 commits into from
Jun 19, 2023
Merged

Reduce Evm Stackspace clearing #5826

merged 32 commits into from
Jun 19, 2023

Conversation

benaadams
Copy link
Member

@benaadams benaadams commented Jun 17, 2023

As there are a large number of CALL/DELEGATECALL/STATICCALL per transaction and each call becomes both a call and call back the ExecuteCall/Code method is executed many times. On currently on entering the method it zeros 9136 bytes of stack.

Some blocks can make up to 4k+ calls meaning 8k+ method entries for these heavy blocks or 70MB. Also the method is 26.5kb of asm which reduces the Jits willingness to optimize and is a large code chunk for a hot loop.

e.g. in the example below 7 blocks execute 724 txs which calls ExecuteCall 10.5k times meaning 92MB of stack needs to be cleared

image

This change reduces to to 1328 bytes of stack or 78MB less in the example above; and reduced the hot loop from 26.5kB to 17.2kB so is more likely to be hotter in the CPU L1 execution cache (32KB - 48KB)

Changes

  • Split ExecuteCall-> ExecuteCall+ExecuteCode; the first being setup and second being hot loop to reduce register pressure on the loop from the setup.
  • Reuse large struct stack variables; as the Jit will allocate a spot in the stack for each declaration
  • Reduce method code/repeats for failures
  • Move Sstore to own method, uses a fair amount of stack variables and not commonly called as %
  • Move Log to own method, uses a fair amount of stack variables and not commonly called as %
  • Move create to own method, uses a fair amount of stack variables and not commonly called as %
  • Move SelfDestruct to own method, uses a fair amount of stack variables and not commonly called (i.e max once)
  • Move Return to own method, uses a fair amount of stack variables and not commonly called (i.e max once)
  • Move Call to own method, uses a fair amount of stack variables and not commonly called (i.e max once)
  • Use struct based Generic parameters and type checks for tracing to effectively remove all the code and the if checks from the compiled Asm when tracing is not enabled (the Jit will generate a new method for each different struct generic parameter, so can use this to make compile time if checks and then remove the branching and dead code)

Types of changes

What types of changes does your code introduce?

  • Bugfix (a non-breaking change that fixes an issue)
  • New feature (a non-breaking change that adds functionality)
  • Breaking change (a change that causes existing functionality not to work as expected)
  • Optimization
  • Refactoring
  • Documentation update
  • Build-related changes
  • Other: Description

Testing

Requires testing

  • Yes
  • No

If yes, did you write tests?

  • Yes
  • No

Notes on testing

Optional. Remove if not applicable.

Remarks

Optional. Remove if not applicable.

@benaadams benaadams marked this pull request as ready for review June 18, 2023 06:02
Copy link
Member

@LukaszRozmej LukaszRozmej left a comment

Choose a reason for hiding this comment

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

We need all tests known to man to run here (hive ect.)

Do you know what is the speedup?

Maybe all the created methods should also have [SkipLocalsInit] ?

src/Nethermind/Nethermind.Evm/VirtualMachine.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.Evm/VirtualMachine.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.Evm/VirtualMachine.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.Evm/VirtualMachine.cs Outdated Show resolved Hide resolved
src/Nethermind/Nethermind.Evm/VirtualMachine.cs Outdated Show resolved Hide resolved
(long)length * GasCostOf.LogData, ref gasAvailable)) return false;

ReadOnlyMemory<byte> data = vmState.Memory.Load(in position, length);
Keccak[] topics = new Keccak[topicsCount];
Copy link
Member

Choose a reason for hiding this comment

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

Idea - Pool those arrays between blocks?


LogEntry logEntry = new(
vmState.Env.ExecutingAccount,
data.ToArray(),
Copy link
Member

Choose a reason for hiding this comment

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

Similar here, maybe we can do some between-block Pool? Need to check all other places in VM.

@benaadams benaadams merged commit 162d666 into master Jun 19, 2023
122 of 130 checks passed
@benaadams benaadams deleted the stackspace branch June 19, 2023 23:14
@natebeauregard natebeauregard mentioned this pull request Apr 25, 2024
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants