Unroll Untraced Execution #4540

shemnon · 2022-10-16T21:29:06Z

PR description

Add a second run-to-halt path used when there is no tracing. This path calls static final versions of select operations with the aim of avoiding the megamorphic dispatch of the original loop.

Fixed Issue(s)

Documentation

I thought about documentation and added the doc-change-required label to this PR if
updates are required.

Changelog

I thought about the changelog and included a changelog update if required.

Add a second run-to-halt path used when there is no tracing. This path calls static final versions of select operations with the aim of avoiding the megamorphic dispatch of the original loop. Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

shemnon · 2022-10-17T16:22:22Z

~20-30% from prior PR,
~41-48% from 22.7.x

The hitch is that these are only for executions that do not involve tracing. This is standard for a mainnet production, but hedera depends on a tracer for some execution. I think I can re-work the trace interface to do what hedera needs.

./gradlew ethereum:evmTool:installDist && ./ethereum/evmtool/build/install/evmtool/bin/evm state-test ethereum/referencetests/src/reference-test/external-resources/GeneralStateTests/VMTests/vmPerformance/loopMul.json

Old (#4533)

{"output":"e2ef8b895ee03259ee57850b1a1ed4aa7689bec352659200e63440303f9d0b81","Mgps":"211.153","gasUsed":"0x96cd6dc2","time":11982032500,"test":"loopMul","fork":"Berlin","d":0,"g":0,"v":0,"postHash":"0x65134195701ec3aa354731a26d9533e946f11e11e60247cc96f57b305778fb12","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":true}
{"output":"ffffffffffffffffffffffffffaaffffffffffffffffbbffffffffffffff0009","Mgps":"235.737","gasUsed":"0x1778312c4","time":26724899042,"test":"loopMul","fork":"Berlin","d":1,"g":0,"v":0,"postHash":"0xa419240a3f3b2a7858212be7e9d6418d8cffd1673dca395582f191621555993d","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":true}
{"output":"0e1c6aac6663c379a52d9ccc7ba4757131020772d41447dfcf478cf9fb0c2bbf","Mgps":"147.324","gasUsed":"0x212496c6","time":3774310042,"test":"loopMul","fork":"Berlin","d":2,"g":0,"v":0,"postHash":"0x0ed14414e692f2ed97cef2b5a500863e2de358b42351d93b963ef4efb3d13d72","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":true}
{"output":"e2ef8b895ee03259ee57850b1a1ed4aa7689bec352659200e63440303f9d0b81","Mgps":"212.748","gasUsed":"0x96cd658e","time":11892231375,"test":"loopMul","fork":"Istanbul","d":0,"g":0,"v":0,"postHash":"0xee92d0c049225c53aa9b156986ad4e7cc9ec2524395c60876a17259fadc8aeae","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":true}
{"output":"ffffffffffffffffffffffffffaaffffffffffffffffbbffffffffffffff0009","Mgps":"235.210","gasUsed":"0x177830a90","time":26784780875,"test":"loopMul","fork":"Istanbul","d":1,"g":0,"v":0,"postHash":"0xe9bca424874ebc2457c9822f6c2f6d2b81ca9194b767c62f9b3c1e554d0a58f3","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":true}
{"output":"0e1c6aac6663c379a52d9ccc7ba4757131020772d41447dfcf478cf9fb0c2bbf","Mgps":"150.844","gasUsed":"0x21248e92","time":3686206708,"test":"loopMul","fork":"Istanbul","d":2,"g":0,"v":0,"postHash":"0xdf31131823f2d621cf2c2d980084681c7ac75a3991480c7d05a7031df34e21c5","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":true}
{"output":"e2ef8b895ee03259ee57850b1a1ed4aa7689bec352659200e63440303f9d0b81","Mgps":"214.435","gasUsed":"0x96cd6dc2","time":11798662084,"test":"loopMul","fork":"London","d":0,"g":0,"v":0,"postHash":"0x662588feffcd4391bc440510de21cbfa8f6c59ceafb2384ed706eccb55ec3a92","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":true}
{"output":"ffffffffffffffffffffffffffaaffffffffffffffffbbffffffffffffff0009","Mgps":"237.000","gasUsed":"0x1778312c4","time":26582431333,"test":"loopMul","fork":"London","d":1,"g":0,"v":0,"postHash":"0x9a8c08120c2517fd2f5c96d377b401258121e3e44b9e5086ac236c6438bfac8e","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":true}
{"output":"0e1c6aac6663c379a52d9ccc7ba4757131020772d41447dfcf478cf9fb0c2bbf","Mgps":"151.160","gasUsed":"0x212496c6","time":3678536750,"test":"loopMul","fork":"London","d":2,"g":0,"v":0,"postHash":"0x221d03f92d6384bd7f947f156244f75262164d483dd1e11ee3f4c32e4030d9fe","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":true}```

New

{"output":"e2ef8b895ee03259ee57850b1a1ed4aa7689bec352659200e63440303f9d0b81","Mgps":"256.707","gasUsed":"0x96cd6dc2","time":9855752125,"test":"loopMul","fork":"Berlin","d":0,"g":0,"v":0,"postHash":"0x65134195701ec3aa354731a26d9533e946f11e11e60247cc96f57b305778fb12","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":true}
{"output":"ffffffffffffffffffffffffffaaffffffffffffffffbbffffffffffffff0009","Mgps":"342.723","gasUsed":"0x1778312c4","time":18382344416,"test":"loopMul","fork":"Berlin","d":1,"g":0,"v":0,"postHash":"0xa419240a3f3b2a7858212be7e9d6418d8cffd1673dca395582f191621555993d","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":true}
{"output":"0e1c6aac6663c379a52d9ccc7ba4757131020772d41447dfcf478cf9fb0c2bbf","Mgps":"187.917","gasUsed":"0x212496c6","time":2958996584,"test":"loopMul","fork":"Berlin","d":2,"g":0,"v":0,"postHash":"0x0ed14414e692f2ed97cef2b5a500863e2de358b42351d93b963ef4efb3d13d72","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":true}
{"output":"e2ef8b895ee03259ee57850b1a1ed4aa7689bec352659200e63440303f9d0b81","Mgps":"297.429","gasUsed":"0x96cd658e","time":8506380541,"test":"loopMul","fork":"Istanbul","d":0,"g":0,"v":0,"postHash":"0xee92d0c049225c53aa9b156986ad4e7cc9ec2524395c60876a17259fadc8aeae","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":true}
{"output":"ffffffffffffffffffffffffffaaffffffffffffffffbbffffffffffffff0009","Mgps":"339.950","gasUsed":"0x177830a90","time":18532281125,"test":"loopMul","fork":"Istanbul","d":1,"g":0,"v":0,"postHash":"0xe9bca424874ebc2457c9822f6c2f6d2b81ca9194b767c62f9b3c1e554d0a58f3","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":true}
{"output":"0e1c6aac6663c379a52d9ccc7ba4757131020772d41447dfcf478cf9fb0c2bbf","Mgps":"187.086","gasUsed":"0x21248e92","time":2972130792,"test":"loopMul","fork":"Istanbul","d":2,"g":0,"v":0,"postHash":"0xdf31131823f2d621cf2c2d980084681c7ac75a3991480c7d05a7031df34e21c5","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":true}
{"output":"e2ef8b895ee03259ee57850b1a1ed4aa7689bec352659200e63440303f9d0b81","Mgps":"295.334","gasUsed":"0x96cd6dc2","time":8566716667,"test":"loopMul","fork":"London","d":0,"g":0,"v":0,"postHash":"0x662588feffcd4391bc440510de21cbfa8f6c59ceafb2384ed706eccb55ec3a92","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":true}
{"output":"ffffffffffffffffffffffffffaaffffffffffffffffbbffffffffffffff0009","Mgps":"329.898","gasUsed":"0x1778312c4","time":19096957625,"test":"loopMul","fork":"London","d":1,"g":0,"v":0,"postHash":"0x9a8c08120c2517fd2f5c96d377b401258121e3e44b9e5086ac236c6438bfac8e","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":true}
{"output":"0e1c6aac6663c379a52d9ccc7ba4757131020772d41447dfcf478cf9fb0c2bbf","Mgps":"189.858","gasUsed":"0x212496c6","time":2928746250,"test":"loopMul","fork":"London","d":2,"g":0,"v":0,"postHash":"0x221d03f92d6384bd7f947f156244f75262164d483dd1e11ee3f4c32e4030d9fe","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":true}

shemnon · 2022-10-17T16:25:53Z

Work isn't done, I should finish unrolling the basic arithmetic that hasn't changed since Frontier. Keeping the operations for complex operations won't have too bad an impact and preserves the customizability of the Besu EVM.

the unrolling is a bit of pick and choose based off of cartevm testing Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

Restore the tracing functionality by splitting it into pre and post execute tracing calls, removing the lambda based wrapping tracer. Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

shemnon · 2022-10-18T16:47:43Z

All the unrolling that adds performance instead of removing performance is in. PR is ready for review.

Overview of changes:

Remove vestigial log tracing. We will never use it in production and standard json tracing gets us what we need
Reduce use of lambdas and optionals. Reads great, translates into a 10% perf hit in a tight loop.
Unroll operation loop in some cases. Those are (a) ops that haven't changed in any way since Frontier (b) ops not overridden in downstream uses and (c) operations that translate into short static executions. This has the longest tendrils as it is enabled by operations exposing static methods to do their work.
Refactoring of the operationTracer. The single, lambda consuming traceExecution method was a barrier to performance. It has been replaced with tracePreExecution and tracePostExecution. Look at DebugOperationTracer to see how traces that need to operate on both sides of the operation can be handled with this API.

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

garyschulte · 2022-10-19T09:00:26Z

evm/src/main/java/org/hyperledger/besu/evm/operation/JumpiOperation.java

@@ -20,9 +20,6 @@
 import org.hyperledger.besu.evm.frame.MessageFrame;
 import org.hyperledger.besu.evm.gascalculator.GasCalculator;

-import java.util.Optional;
-import java.util.OptionalLong;
-
 import org.apache.tuweni.bytes.Bytes;

 public class JumpiOperation extends AbstractFixedCostOperation {


Was there not a similar benefit to a static OperationResult(xx,null) success response for the Jump operations?

no, there wasn't. I was suprised.

garyschulte · 2022-10-19T09:10:09Z

evm/src/main/java/org/hyperledger/besu/evm/operation/BaseFeeOperation.java

@@ -34,8 +33,7 @@ public Operation.OperationResult executeFixedCostOperation(
      final MessageFrame frame, final EVM evm) {
    final Optional<Wei> maybeBaseFee = frame.getBlockValues().getBaseFee();
    if (maybeBaseFee.isEmpty()) {
-      return new Operation.OperationResult(
-          OptionalLong.of(gasCost), Optional.of(ExceptionalHaltReason.INVALID_OPERATION));
+      return new Operation.OperationResult(gasCost, ExceptionalHaltReason.INVALID_OPERATION);
    }
    frame.pushStackItem(maybeBaseFee.orElseThrow());
    return successResponse;


same q about static OperationResult(2, null) here

Basefee gets overridden in downstream clients, so it cannot be optimized.

garyschulte · 2022-10-19T09:14:09Z

evm/src/main/java/org/hyperledger/besu/evm/operation/AddOperation.java

@@ -46,4 +48,21 @@ public Operation.OperationResult executeFixedCostOperation(

    return successResponse;
  }
+
+  public static OperationResult staticOperation(final MessageFrame frame) {


Why are we not calling this from executeFixedCostOperation ? It looks like the EVM will call staticOperation from the case statement, so this is just a question of consistency

garyschulte

non-blocking feedback and questions, but looks great. It would be great to get this into the RC2 release today.

One other question - have you verified that the pre/post stack change to tracing API results in no tracing output diffs?

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

shemnon · 2022-10-19T13:47:52Z

non-blocking feedback and questions, but looks great. It would be great to get this into the RC2 release today.

One other question - have you verified that the pre/post stack change to tracing API results in no tracing output diffs?

Yes, we have unit tests for the tracers and it comes out the same. That was what two hours of the work time was spent doing, figuring out what fields had to be read pre-exec and stored in the tracer and what is read post-exec for the output.

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

Overview of changes: * Remove vestigial log tracing. We will never use it in production and standard json tracing gets us what we need * Reduce use of lambdas and optionals. Reads great, translates into a 10% perf hit in a tight loop. * Unroll operation loop in some cases. Those are (a) ops that haven't changed in any way since Frontier (b) ops not overridden in downstream uses and (c) operations that translate into short static executions. This has the longest tendrils as it is enabled by operations exposing static methods to do their work. * Refactoring of the operationTracer. The single, lambda consuming traceExecution method was a barrier to performance. It has been replaced with tracePreExecution and tracePostExecution. Look at DebugOperationTracer to see how traces that need to operate on both sides of the operation can be handled with this API. Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com> Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>

Overview of changes: * Remove vestigial log tracing. We will never use it in production and standard json tracing gets us what we need * Reduce use of lambdas and optionals. Reads great, translates into a 10% perf hit in a tight loop. * Unroll operation loop in some cases. Those are (a) ops that haven't changed in any way since Frontier (b) ops not overridden in downstream uses and (c) operations that translate into short static executions. This has the longest tendrils as it is enabled by operations exposing static methods to do their work. * Refactoring of the operationTracer. The single, lambda consuming traceExecution method was a barrier to performance. It has been replaced with tracePreExecution and tracePostExecution. Look at DebugOperationTracer to see how traces that need to operate on both sides of the operation can be handled with this API. Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

shemnon added 5 commits October 16, 2022 15:28

Unroll Untraced Execution

a781365

Add a second run-to-halt path used when there is no tracing. This path calls static final versions of select operations with the aim of avoiding the megamorphic dispatch of the original loop. Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

Looks like I didn't compile tests

35624c5

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

spotless

41036f2

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

unit test breaks

e51b1b0

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

spotless

bceb090

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

shemnon added 4 commits October 18, 2022 08:37

more unrolling

b143dde

the unrolling is a bit of pick and choose based off of cartevm testing Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

Restore Tracing

87ac0e8

Restore the tracing functionality by splitting it into pre and post execute tracing calls, removing the lambda based wrapping tracer. Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

undo local build changes

34b3b08

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

null for performance

fc23c20

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

shemnon marked this pull request as ready for review October 18, 2022 16:47

shemnon added 2 commits October 18, 2022 10:48

Merge branch 'main' of github.com:hyperledger/besu into perf-evm-unroll

9b6d2b7

changelog

b0e63fa

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

garyschulte reviewed Oct 19, 2022

View reviewed changes

garyschulte approved these changes Oct 19, 2022

View reviewed changes

use static add.

fbf7d6f

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

merge

a2968f7

Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>

shemnon enabled auto-merge (squash) October 19, 2022 13:52

Merge branch 'main' into perf-evm-unroll

06ed30a

shemnon merged commit 59d3044 into hyperledger:main Oct 19, 2022

shemnon mentioned this pull request Nov 7, 2022

Migrate EVM to 22.10.x hashgraph/hedera-services#4120

Merged

2 tasks

NickSneo mentioned this pull request Feb 21, 2023

Fixes different gas consumption by Besu when compared to Geth #5116

Closed

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unroll Untraced Execution #4540

Unroll Untraced Execution #4540

shemnon commented Oct 16, 2022 •

edited

Loading

shemnon commented Oct 17, 2022 •

edited

Loading

shemnon commented Oct 17, 2022

shemnon commented Oct 18, 2022

garyschulte Oct 19, 2022 •

edited

Loading

shemnon Oct 19, 2022

garyschulte Oct 19, 2022

shemnon Oct 19, 2022

garyschulte Oct 19, 2022 •

edited

Loading

garyschulte left a comment

shemnon commented Oct 19, 2022

Unroll Untraced Execution #4540

Unroll Untraced Execution #4540

Conversation

shemnon commented Oct 16, 2022 • edited Loading

PR description

Fixed Issue(s)

Documentation

Changelog

shemnon commented Oct 17, 2022 • edited Loading

shemnon commented Oct 17, 2022

shemnon commented Oct 18, 2022

garyschulte Oct 19, 2022 • edited Loading

Choose a reason for hiding this comment

shemnon Oct 19, 2022

Choose a reason for hiding this comment

garyschulte Oct 19, 2022

Choose a reason for hiding this comment

shemnon Oct 19, 2022

Choose a reason for hiding this comment

garyschulte Oct 19, 2022 • edited Loading

Choose a reason for hiding this comment

garyschulte left a comment

Choose a reason for hiding this comment

shemnon commented Oct 19, 2022

shemnon commented Oct 16, 2022 •

edited

Loading

shemnon commented Oct 17, 2022 •

edited

Loading

garyschulte Oct 19, 2022 •

edited

Loading

garyschulte Oct 19, 2022 •

edited

Loading