Add EVM profiler #2988

jochem-brouwer · 2023-08-24T14:42:48Z

This PR adds the feature for an EVM profiler. It features, once enabled, tracking:

The amount of calls, the time it takes, and the gas it consumes for each opcode
The same "inside" the precompiles

For precompiles we have the special case that the gas there is not tracked as in normal EVM calls, but instead depends upon what precompile is called. If a precompile is called, the "inner" gas used by that precompile (so without the overhead of the initial CALL opcode to the precompile) is tracked.

The VM gets extra options: profilerOpts, which can reportProfilerAfterBlock and reportProfilerAfterTx. Note that if a profiler is reported, the profiler in the EVM is cleared, so you cannot use both options.

EVM itself has an extra option as well, the profiler (should rename this to profilerOpts as well for consistency) which currently only has the enable flag. This will later support reporting the gas used for different hardforks (while the actual EVM runs on the hardfork it expects for that call) such that we can simulate running Shanghai gas on older blocks (since EVM had many underpriced situations back then).

To run and test:

Add this to BlockchainTestRunner:

  let vm = await VM.create({
    stateManager,
    blockchain,
    common,
    setHardfork: true,
    profilerOpts: {
      reportProfilerAfterBlock: true
    }
  })

Now run a blockchain test: npm run test:blockchain -- --fork=Shanghai --test=coinbaseWarmAccountCallGasFail_d3g0v0_Shanghai

Example output:

TODOs

Gas reported for CALL situations is incorrect, this encompasses setting up the CALL, but also all time consumed within that call frame
- To test: DEBUG=ethjs,evm:profiler npm run test:blockchain -- --fork=Shanghai --test=ContractCreationSpam_d0g0v0_Shanghai
- The total time of each opcode consumed is higher than the total test run
Test if precompile reporting is correct

TODOs for subsequent PRs

Add option to set custom hardfork for gas reporting to EVM
Add option to also profile the pre-EVM-call setup in VM, and also the post-EVM-call setup in VM
Add option for blockchain/state tests to --profile them

codecov · 2023-08-24T14:47:17Z

Codecov Report

Merging #2988 (de824c8) into master (f5c6769) will decrease coverage by 0.37%.
The diff coverage is 47.72%.

Additional details and impacted files

Flag	Coverage Δ
block	`88.73% <ø> (ø)`
blockchain	`92.58% <ø> (ø)`
client	`87.39% <100.00%> (-0.06%)`	⬇️
common	`98.18% <ø> (ø)`
ethash	`∅ <ø> (∅)`
evm	`69.75% <56.60%> (-0.68%)`	⬇️
rlp	`∅ <ø> (?)`
statemanager	`84.46% <ø> (ø)`
trie	`90.07% <ø> (+0.25%)`	⬆️
tx	`96.38% <ø> (ø)`
util	`86.78% <ø> (ø)`
vm	`75.97% <16.36%> (-3.23%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

holgerd77 · 2023-08-24T14:50:18Z

packages/vm/src/types.ts

+  profilerOpts?: {
+    //evmProfilerOpts: EVMProfilerOpts
+    reportProfilerAfterTx?: boolean
+    reportProfilerAfterBlock?: boolean


Two quick nits: I would drop the Profiler here from the two options, so just reportAfterTx and reportAfterBlock, context is already clear and option gets shorter.

I have explicitly added this profiler to VM, because we want to (?) profile VM pre-setup-before EVM call and post-setup-after-EVM call too. So after the EVM is ran, then VM will flush the StateManager and this can take some time in some situations, which we want to profile to (?)

This was just about naming the options (please re-read in this context), so removing Profiler from the options names since it's redundant and makes the names long. 🙂

Ah you are right, will update

I misread your comment, I thought you meant to remove profilerOpts and put those options directly into (so without an encapsulating object)

I would actually still be a fan of this name changes (can be along some follow-up work), so that we get to e.g. this._opts.profilerOpts?.reportAfterBlock (still long enough 🙂).

Ok, addressed

I would actually still be a fan of this name changes (can be along some follow-up work), so that we get to e.g. this._opts.profilerOpts?.reportAfterBlock (still long enough 🙂).

Yes, added this :)

jochem-brouwer · 2023-08-28T11:25:30Z

holgerd77 · 2023-08-29T11:50:10Z

packages/client/bin/cli.ts

+    describe: 'Report the VM profile after running each block',
+    boolean: true,
+    default: false,
+  })


Oh this is cool! 🤩

Didn't know that you had integrated so deeply into client already...

Yes, the only thing not to forget is; it does not work if you don't set the DEBUG=ethjs,evm:profiler env, so this is something we can improve (but not sure how to do this cleanly)

You should just be able to set the debug variable programmatically with this process.env.DEBUG = 'ethjs,evm:profiler'. So just add a code block to the run command in the client that checks for either of these args and then sets the debug accordingly. Or better, do something like this:

if (args.vmProfileBlocks || args.vmProfileTxs) { let debugString = process.env.DEBUG if debugString.startsWith('ethjs') { debugString = debugString + ',evm:profiler' } else { debugString = 'ethjs,evm:profiler' } process.env.DEBUG=debugString }

Note, I haven't tested the above code but I do something like this in one of the trie demos to force it to output certain debug logs.

jochem-brouwer · 2023-08-30T19:29:00Z

HEAD at 2e06d87 (backup)

acolytec3

This looks good, though left a couple of questions about syntax I'm not quite tracking.

acolytec3 · 2023-08-30T19:39:30Z

packages/evm/src/interpreter.ts

@@ -171,6 +177,7 @@ export class Interpreter {
      returnValue: undefined,
      selfdestruct: new Set(),
    }
+    ;(this.profilerOpts = profilerOpts), (this.performanceLogger = performanceLogs)


What is this syntax?

No idea 😂 I definitely did not write this, I think I made a weird syntax mistake and then the linter did this. Will fix

packages/evm/src/interpreter.ts

acolytec3

LGTM

holgerd77

LGTM

holgerd77

Still looks good to me 🙂

holgerd77 · 2023-08-30T20:37:21Z

🎉

evm/vm: add optional evm profiler

567f7c8

jochem-brouwer added PR state: WIP type: performance package: vm package: evm labels Aug 24, 2023

holgerd77 reviewed Aug 24, 2023

View reviewed changes

holgerd77 reviewed Aug 29, 2023

View reviewed changes

jochem-brouwer marked this pull request as ready for review August 30, 2023 16:22

jochem-brouwer added PR state: needs review and removed PR state: WIP labels Aug 30, 2023

jochem-brouwer mentioned this pull request Aug 30, 2023

Add EVM profiler v2 #2998

Closed

jochem-brouwer added 2 commits August 30, 2023 20:12

fix submodule delete

7a47482

Merge branch 'master' into evm-performance-logger-sm3

f81882e

jochem-brouwer mentioned this pull request Aug 30, 2023

Add EVM profiler V3 #2999

Closed

jochem-brouwer force-pushed the evm-performance-logger branch from 2e06d87 to f81882e Compare August 30, 2023 19:30

acolytec3 reviewed Aug 30, 2023

View reviewed changes

jochem-brouwer commented Aug 30, 2023

View reviewed changes

packages/evm/src/interpreter.ts Show resolved Hide resolved

fix syntax

4768496

acolytec3 previously approved these changes Aug 30, 2023

View reviewed changes

acolytec3 and others added 2 commits August 30, 2023 15:56

Merge branch 'master' into evm-performance-logger

07f5dd9

vm: update logger option names

7a337c8

jochem-brouwer dismissed acolytec3’s stale review via 7a337c8 August 30, 2023 20:05

holgerd77 previously approved these changes Aug 30, 2023

View reviewed changes

client: fix performance opts

de824c8

jochem-brouwer dismissed holgerd77’s stale review via de824c8 August 30, 2023 20:11

holgerd77 approved these changes Aug 30, 2023

View reviewed changes

jochem-brouwer merged commit 3c92910 into master Aug 30, 2023
41 of 42 checks passed

holgerd77 deleted the evm-performance-logger branch August 30, 2023 20:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add EVM profiler #2988

Add EVM profiler #2988

jochem-brouwer commented Aug 24, 2023 •

edited

Loading

codecov bot commented Aug 24, 2023 •

edited

Loading

holgerd77 Aug 24, 2023

jochem-brouwer Aug 24, 2023

holgerd77 Aug 30, 2023

jochem-brouwer Aug 30, 2023

jochem-brouwer Aug 30, 2023

holgerd77 Aug 30, 2023

jochem-brouwer Aug 30, 2023

jochem-brouwer Aug 30, 2023

jochem-brouwer commented Aug 28, 2023

holgerd77 Aug 29, 2023

holgerd77 Aug 29, 2023

jochem-brouwer Aug 29, 2023

acolytec3 Aug 29, 2023

acolytec3 Aug 29, 2023

jochem-brouwer commented Aug 30, 2023 •

edited

Loading

acolytec3 left a comment

acolytec3 Aug 30, 2023

jochem-brouwer Aug 30, 2023

acolytec3 left a comment

holgerd77 left a comment

holgerd77 left a comment

holgerd77 commented Aug 30, 2023

Add EVM profiler #2988

Add EVM profiler #2988

Conversation

jochem-brouwer commented Aug 24, 2023 • edited Loading

codecov bot commented Aug 24, 2023 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jochem-brouwer commented Aug 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jochem-brouwer commented Aug 30, 2023 • edited Loading

acolytec3 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acolytec3 left a comment

Choose a reason for hiding this comment

holgerd77 left a comment

Choose a reason for hiding this comment

holgerd77 left a comment

Choose a reason for hiding this comment

holgerd77 commented Aug 30, 2023

jochem-brouwer commented Aug 24, 2023 •

edited

Loading

codecov bot commented Aug 24, 2023 •

edited

Loading

jochem-brouwer commented Aug 30, 2023 •

edited

Loading