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

eth/tracers: support for golang tracers + add golang callTracer #23708

Merged
merged 31 commits into from Nov 5, 2021

Conversation

s1na
Copy link
Contributor

@s1na s1na commented Oct 11, 2021

Implements a native call tracer along with a way to register such native tracers. Custom tracers can be implemented that adhere to the interface and dropped in ./eth/tracers/native while calling Register(name, constructorFn) in their init function.

@s1na
Copy link
Contributor Author

s1na commented Oct 12, 2021

Weird that for some cases it's slower than the new js call tracer:

Tracers/create-8                      29.5µs ± 1%    41.2µs ± 1%   +39.98%  (p=0.008 n=5+5)
Tracers/deepCalls-8                   3.57ms ± 6%    0.35ms ± 0%   -90.22%  (p=0.016 n=5+4)
Tracers/delegatecall-8                 256µs ± 2%      41µs ± 0%   -84.09%  (p=0.008 n=5+5)
Tracers/innerCreateOogOuterThrow-8     220µs ± 1%      91µs ± 1%   -58.87%  (p=0.008 n=5+5)
Tracers/innerInstafail-8              9.37µs ± 2%   12.30µs ± 2%   +31.17%  (p=0.008 n=5+5)
Tracers/innerThrowOuterRevert-8        158µs ± 1%      38µs ± 1%   -75.80%  (p=0.008 n=5+5)
Tracers/oog-8                         19.9µs ± 1%    23.3µs ± 0%   +17.04%  (p=0.008 n=5+5)
Tracers/revert-8                      10.8µs ±12%    12.2µs ± 1%   +12.83%  (p=0.016 n=5+5)
Tracers/revertReason-8                17.3µs ± 1%    18.0µs ± 1%    +4.48%  (p=0.008 n=5+5)
Tracers/selfdestruct-8                 242µs ± 2%      18µs ± 1%   -92.45%  (p=0.008 n=5+5)
Tracers/simple-8                       235µs ± 2%      25µs ± 0%   -89.19%  (p=0.008 n=5+5)
Tracers/throw-8                       17.7µs ± 2%    18.2µs ± 0%    +3.03%  (p=0.016 n=5+4)

Edit: seems like allocations are higher for the slow cases:

Tracers/create-8                      8.97kB ± 0%   33.73kB ± 0%  +275.84%  (p=0.000 n=10+10)
Tracers/deepCalls-8                    113kB ± 0%     115kB ± 0%    +1.34%  (p=0.000 n=10+10)
Tracers/delegatecall-8                12.1kB ± 0%    14.0kB ± 0%   +16.31%  (p=0.000 n=10+10)
Tracers/innerCreateOogOuterThrow-8     113kB ± 0%      91kB ± 0%   -19.42%  (p=0.000 n=10+10)
Tracers/innerInstafail-8              3.42kB ± 0%    5.74kB ± 0%   +68.15%  (p=0.000 n=10+10)
Tracers/innerThrowOuterRevert-8       8.51kB ± 0%   10.54kB ± 0%   +23.89%  (p=0.000 n=10+6)
Tracers/oog-8                         4.07kB ± 0%    6.57kB ± 0%   +61.36%  (p=0.000 n=8+10)
Tracers/revert-8                      2.93kB ± 0%    6.85kB ± 0%  +133.95%  (p=0.000 n=10+9)
Tracers/revertReason-8                4.57kB ± 0%    7.40kB ± 0%   +62.03%  (p=0.000 n=9+9)
Tracers/selfdestruct-8                7.16kB ± 0%    9.21kB ± 0%   +28.53%  (p=0.000 n=9+10)
Tracers/simple-8                      7.18kB ± 0%    9.58kB ± 0%   +33.49%  (p=0.000 n=10+10)
Tracers/throw-8                       3.93kB ± 0%    6.17kB ± 0%   +56.99%  (p=0.000 n=10+9)

Edit2: To my surprise even commenting out all of the native call tracer's logic leads to 8776 bytes alloc in Tracers/create which is equivalent to that of the JS call tracer. I'm starting to think the allocs reported for the JS tracer are not accurate, maybe because of cgo?

@s1na s1na marked this pull request as ready for review October 12, 2021 15:04
@s1na
Copy link
Contributor Author

s1na commented Oct 13, 2021

The benchmark wasn't doing GetResult and that made the difference because the native tracer does some work on start which js tracer does only in result. Now we're talking:

name                                old time/op    new time/op    delta
Tracers/create-8                      7.86ms ± 4%    0.06ms ±11%  -99.26%  (p=0.008 n=5+5)
Tracers/deepCalls-8                   11.6ms ± 1%     0.4ms ± 1%  -96.69%  (p=0.008 n=5+5)
Tracers/delegatecall-8                7.65ms ± 3%    0.05ms ± 2%  -99.37%  (p=0.008 n=5+5)
Tracers/innerCreateOogOuterThrow-8    8.04ms ± 0%    0.12ms ± 1%  -98.49%  (p=0.016 n=4+5)
Tracers/innerInstafail-8              7.70ms ± 5%    0.02ms ± 4%  -99.78%  (p=0.008 n=5+5)
Tracers/innerThrowOuterRevert-8       7.64ms ± 3%    0.04ms ± 0%  -99.41%  (p=0.008 n=5+5)
Tracers/oog-8                         7.72ms ± 1%    0.03ms ± 1%  -99.63%  (p=0.008 n=5+5)
Tracers/revert-8                      7.62ms ± 1%    0.02ms ± 1%  -99.77%  (p=0.008 n=5+5)
Tracers/revertReason-8                7.78ms ± 9%    0.02ms ± 1%  -99.70%  (p=0.008 n=5+5)
Tracers/selfdestruct-8                7.58ms ± 2%    0.02ms ± 3%  -99.69%  (p=0.008 n=5+5)
Tracers/simple-8                      7.54ms ± 1%    0.03ms ± 3%  -99.59%  (p=0.008 n=5+5)
Tracers/throw-8                       7.47ms ± 4%    0.02ms ± 2%  -99.70%  (p=0.008 n=5+5)

@holiman
Copy link
Contributor

holiman commented Oct 13, 2021

🤯

@holiman
Copy link
Contributor

holiman commented Oct 13, 2021

But wait. What are you comparing to?
The current js tracer should already be in the sub-ms region: #23087 (comment)

EDIT: Ah, I see, the result was deferred. I agre it should be included in the benchmark, yes, the way you've done it here

eth/tracers/native/call.go Outdated Show resolved Hide resolved
eth/tracers/native/call.go Outdated Show resolved Hide resolved
eth/tracers/native/native.go Outdated Show resolved Hide resolved
eth/tracers/native/native.go Outdated Show resolved Hide resolved
Copy link
Contributor

@holiman holiman left a comment

Choose a reason for hiding this comment

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

LGTM!

@holiman holiman changed the title Native call tracer eth/tracers: support for golang tracers + add golang callTracer Nov 4, 2021
@holiman
Copy link
Contributor

holiman commented Nov 4, 2021

The native tracer, when doing calltracer, took 2 hours to reach the point where it could swap out the mem-trie to a disk-trie

Screenshot 2021-11-04 at 22-49-38 Single Geth - Grafana

Previously, when I did the native no-op-tracer, it was almost the same time -- a bit less than 2 hours until the mem-drop. That's pretty cool, it means that the tracecall overhead is almost negligible for the native tracer.

@holiman
Copy link
Contributor

holiman commented Nov 4, 2021

native call-tracer results:

INFO [11-04|23:54:59.040] Chain tracing finished                   start=13,401,259 end=13,431,259 transactions=5,786,835 elapsed=4h25m6.580980692s

@holiman
Copy link
Contributor

holiman commented Nov 5, 2021

Some more charts

Screenshot 2021-11-05 at 08-27-04 Single Geth - Grafana

The run is not finished yet, but looks like a factor 2.5 speedup between native calltracer vs duktape calltracer (started at midnight). First native-memdrop after 2 hours, first js-memdrop after 4h30m.

@holiman
Copy link
Contributor

holiman commented Nov 5, 2021

Doing some comparisons. Here's the first difference I found

native:
00000000000000000000000000000000000000000000000","output":"0x","error":"execution reverted"}},{"resu
jscript:
00000000000000000000000000000000000000000000000","error":"execution reverted"}},{"result":{"type":"C

@holiman
Copy link
Contributor

holiman commented Nov 5, 2021

And we have the results in:

INFO [11-04|23:54:59.040] Chain tracing finished                   start=13,401,259 end=13,431,259 transactions=5,786,835 elapsed=4h25m6.580980692s

vs js:

INFO [11-05|10:05:46.920] Chain tracing finished                   start=13,401,259 end=13,431,259 transactions=5,786,835 elapsed=10h0m33.057605432s

4h25 vs 10h0m

Comment on lines +83 to +87
if err.Error() == "execution reverted" && len(output) > 0 {
t.callstack[0].Output = bytesToHex(output)
}
} else {
t.callstack[0].Output = bytesToHex(output)
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM. However, apparently this is not covered by the tests, think you can add a test that flags this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not required for this PR though

@holiman
Copy link
Contributor

holiman commented Nov 5, 2021

For the most part, the diffs seem to be of the type

{
    ...skipped 1 object property...,
    "params": {
        "result": {
            "traces": [
                ...skipped 179 array elements...,
                {
                    "result": {
                        ...skipped 6 object properties...,
                        "output": "0x" => "",
                        ...skipped 3 object properties...
                    }
                },
                ...skipped 29 array elements...
            ]
        }
    }
}

But if I work around it by applying your patch in the parser, it works fine (it's gone through 20K blocks now, comparing the outputs)

@holiman holiman merged commit 8d7e606 into ethereum:master Nov 5, 2021
@holiman holiman added this to the 1.10.12 milestone Nov 5, 2021
sidhujag pushed a commit to syscoin/go-ethereum that referenced this pull request Nov 5, 2021
…reum#23708)

* eth/tracers: add basic native loader

* eth/tracers: add GetResult to tracer interface

* eth/tracers: add native call tracer

* eth/tracers: fix call tracer json result

* eth/tracers: minor fix

* eth/tracers: fix

* eth/tracers: fix benchTracer

* eth/tracers: test native call tracer

* eth/tracers: fix

* eth/tracers: rm extra make

Co-authored-by: Martin Holst Swende <martin@swende.se>

* eth/tracers: rm extra make

* eth/tracers: make callFrame private

* eth/tracers: clean-up and comments

* eth/tracers: add license

* eth/tracers: rework the model a bit

* eth/tracers: move tracecall tests to subpackage

* cmd/geth: load native tracers

* eth/tracers: minor fix

* eth/tracers: impl stop

* eth/tracers: add native noop tracer

* renamings

Co-authored-by: Martin Holst Swende <martin@swende.se>

* eth/tracers: more renamings

* eth/tracers: make jstracer non-exported, avoid cast

* eth/tracers, core/vm: rename vm.Tracer to vm.EVMLogger for clarity

* eth/tracers: minor comment fix

* eth/tracers/testing: lint nitpicks

* core,eth: cancel evm on nativecalltracer stop

* Revert "core,eth: cancel evm on nativecalltracer stop"

This reverts commit 01bb908.

* eth/tracers: linter nits

* eth/tracers: fix output on err

Co-authored-by: Martin Holst Swende <martin@swende.se>
@FeurJak
Copy link

FeurJak commented Nov 8, 2021

I have to say that this is some amazing work.. My CallTracer calls have gone down now from avg. ~20ms to avg. ~6ms when using the native Call Tracer. You guys are on another level.

maoueh pushed a commit to streamingfast/go-ethereum that referenced this pull request Jan 31, 2022
* eth/tracers: implement debug.intermediateRoots (ethereum#23594)

This PR implements a new debug method, which I've talked briefly about to some other client developers. It allows the caller to obtain the intermediate state roots for a block (which might be either a canon block or a 'bad' block).
Signed-off-by: wenbiao <delweng@gmail.com>

* core, rpc: disable memory output by default in traces (ethereum#23558)

* core: cmd: invert disableMemory

* core: fix missed inversion

* cmd/evm: preserve Flags but change default value

* Apply suggestions from code review

Co-authored-by: Martin Holst Swende <martin@swende.se>

Co-authored-by: Martin Holst Swende <martin@swende.se>
Signed-off-by: wenbiao <delweng@gmail.com>

* eth/tracers: abort evm execution when trace is aborted (ethereum#23580)

Signed-off-by: wenbiao <delweng@gmail.com>

* eth/tracers: avoid unsyncronized mutations on trie database (ethereum#23632)

This PR fixes an issue in traceChain, where the statedb Commit operation was performed asynchronously with dereference-operations agains the underlying trie.Database instance. Due to how the reference counting works within the trie database (where parent count is recursively updated when new parents are added), doing dereferencing in the middle of Commit can cause the refcount to become wrong, leading to an inconsistent state. 

This was fixed by doing Commit/Deref from the same routine.  
Signed-off-by: wenbiao <delweng@gmail.com>

* core,eth: call frame tracing (ethereum#23087)

This change introduces 2 new optional methods; `enter()` and `exit()` for js tracers, and makes `step()` optiona. The two new methods are invoked when entering and exiting a call frame (but not invoked for the outermost scope, which has it's own methods). Currently these are the data fields passed to each of them:

    enter: type (opcode), from, to, input, gas, value
    exit: output, gasUsed, error

The PR also comes with a re-write of the callTracer. As a backup we keep the previous tracing script under the name `callTracerLegacy`. Behaviour of both tracers are equivalent for the most part, although there are some small differences (improvements), where the new tracer is more correct / has more information.

Signed-off-by: wenbiao <delweng@gmail.com>

* eth/tracers: re-write of 4byte tracer using enter/exit (ethereum#23622)

* eth/tracers: add re-write of 4byte tracer using enter/exit

* eth/tracers: fix 4byte indent
Signed-off-by: wenbiao <delweng@gmail.com>

* eth/tracers: tx.BaseFee not implemented

Signed-off-by: wenbiao <delweng@gmail.com>

* eth/tracers: do the JSON serialization via .js to capture C faults

Signed-off-by: wenbiao <delweng@gmail.com>

* eth/tracers: fix callTracer fault handling (ethereum#23667)

* eth/tracers: fix calltracer fault handling

* eth/tracers: fix calltracer indentation
Signed-off-by: wenbiao <delweng@gmail.com>

* eth/tracers: invoke enter/exit on 0-value calls to inex accounts (ethereum#23828)

Signed-off-by: wenbiao <delweng@gmail.com>

* eth: make traceChain avoid OOM on long-running tracing (ethereum#23736)

This PR changes long-running chain tracing, so that it at some points releases the memory trie db, and switch over to a fresh disk-backed trie.
Signed-off-by: wenbiao <delweng@gmail.com>

* eth/tracers: expose contextual infos (block hash, tx hash, tx index)

Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io>

* eth/tracers: redefine Context

Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io>

* eth/tracers: support for golang tracers + add golang callTracer (ethereum#23708)

* eth/tracers: add basic native loader

* eth/tracers: add GetResult to tracer interface

* eth/tracers: add native call tracer

* eth/tracers: fix call tracer json result

* eth/tracers: minor fix

* eth/tracers: fix

* eth/tracers: fix benchTracer

* eth/tracers: test native call tracer

* eth/tracers: fix

* eth/tracers: rm extra make

Co-authored-by: Martin Holst Swende <martin@swende.se>

* eth/tracers: rm extra make

* eth/tracers: make callFrame private

* eth/tracers: clean-up and comments

* eth/tracers: add license

* eth/tracers: rework the model a bit

* eth/tracers: move tracecall tests to subpackage

* cmd/geth: load native tracers

* eth/tracers: minor fix

* eth/tracers: impl stop

* eth/tracers: add native noop tracer

* renamings

Co-authored-by: Martin Holst Swende <martin@swende.se>

* eth/tracers: more renamings

* eth/tracers: make jstracer non-exported, avoid cast

* eth/tracers, core/vm: rename vm.Tracer to vm.EVMLogger for clarity

* eth/tracers: minor comment fix

* eth/tracers/testing: lint nitpicks

* core,eth: cancel evm on nativecalltracer stop

* Revert "core,eth: cancel evm on nativecalltracer stop"

This reverts commit 01bb908.

* eth/tracers: linter nits

* eth/tracers: fix output on err

Co-authored-by: Martin Holst Swende <martin@swende.se>
Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io>

* eth/tracers: make native calltracer default (ethereum#23867)

Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io>

* eth/tracers: package restructuring (ethereum#23857)

* eth/tracers: restructure tracer package

* core/vm/runtime: load js tracers

* eth/tracers: mv bigint js code to own file

* eth/tracers: add method docs for native tracers

* eth/tracers: minor doc fix

* core,eth: cancel evm on nativecalltracer stop

* core/vm: fix failing test

Co-authored-by: Sina Mahmoodi <itz.s1na@gmail.com>
Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io>

* eth/tracers: ethapi.TransactionArgs was not merged

Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io>

* eth/tracers: fix the api_test with ErrInsufficientFunds to ErrInsufficientFundsForTransfer

Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io>

* eth/tracers: check posa before statedb.Prepare in IntermiateRoots api

Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io>

* eth/tracers: make js calltracer default, compatible with old version

Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io>

* eth/tracers: fix the default callTrace name of callTracerJs

Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io>

* Revert "eth/tracers: fix the default callTrace name of callTracerJs"

This reverts commit 62a3bc215d9f07e422a4c659289bb3ba4f9ed2fa.

Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io>

* Revert "eth/tracers: make js calltracer default, compatible with old version"

This reverts commit 85ef42c0ea651f0b228d4209b1b2598b24e12f1f.

Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io>

* eth/tracers: fix the variable race condition

Signed-off-by: wenbiao <wenbiao.zheng@ambergroup.io>

Co-authored-by: Martin Holst Swende <martin@swende.se>
Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de>
Co-authored-by: Sina Mahmoodi <1591639+s1na@users.noreply.github.com>
Co-authored-by: Péter Szilágyi <peterke@gmail.com>
Co-authored-by: Sina Mahmoodi <itz.s1na@gmail.com>
yongjun925 pushed a commit to DODOEX/go-ethereum that referenced this pull request Dec 3, 2022
…reum#23708)

* eth/tracers: add basic native loader

* eth/tracers: add GetResult to tracer interface

* eth/tracers: add native call tracer

* eth/tracers: fix call tracer json result

* eth/tracers: minor fix

* eth/tracers: fix

* eth/tracers: fix benchTracer

* eth/tracers: test native call tracer

* eth/tracers: fix

* eth/tracers: rm extra make

Co-authored-by: Martin Holst Swende <martin@swende.se>

* eth/tracers: rm extra make

* eth/tracers: make callFrame private

* eth/tracers: clean-up and comments

* eth/tracers: add license

* eth/tracers: rework the model a bit

* eth/tracers: move tracecall tests to subpackage

* cmd/geth: load native tracers

* eth/tracers: minor fix

* eth/tracers: impl stop

* eth/tracers: add native noop tracer

* renamings

Co-authored-by: Martin Holst Swende <martin@swende.se>

* eth/tracers: more renamings

* eth/tracers: make jstracer non-exported, avoid cast

* eth/tracers, core/vm: rename vm.Tracer to vm.EVMLogger for clarity

* eth/tracers: minor comment fix

* eth/tracers/testing: lint nitpicks

* core,eth: cancel evm on nativecalltracer stop

* Revert "core,eth: cancel evm on nativecalltracer stop"

This reverts commit 01bb908.

* eth/tracers: linter nits

* eth/tracers: fix output on err

Co-authored-by: Martin Holst Swende <martin@swende.se>
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.

None yet

6 participants