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

all: fix issues with benchmarks #30667

Merged
merged 9 commits into from
Nov 4, 2024
Merged

Conversation

holiman
Copy link
Contributor

@holiman holiman commented Oct 24, 2024

This PR fixes some issues with benchmarks

  • Removes log output from a log-test
  • Avoids a nil-defer in triedb/pathdb
  • Fixes some crashes re tracers
  • Refactors a very resource-expensive benchmark for blobpol. NOTE: this rewrite touches live production code (a little bit), as it makes the validator-function used by the blobpool configurable.
  • Switch some benches over to use pebble over leveldb
  • reduce mem overhead in the setup-phase of some tests
  • Marks some tests with a long setup-phase to be skipped if -short is specified (where long is on the order of tens of seconds). Ideally, in my opinion, one should be able to run with -benchtime 10ms -short and sanity-check all tests very quickly.
  • Drops some metrics-bechmark which times the speed of copy.

@holiman holiman changed the title log, tridb/pathdb: fix bencharks log, triedb/pathdb: fix bencharks Oct 24, 2024
@holiman holiman changed the title log, triedb/pathdb: fix bencharks log, triedb/pathdb: fix benchmarks Oct 24, 2024
@holiman
Copy link
Contributor Author

holiman commented Oct 24, 2024

@s1na could you please PTAL at BenchmarkTransactionTrace and BenchmarkTracerStepVsCallFrame ? Not sure where/how to fix it properly

@s1na
Copy link
Contributor

s1na commented Oct 24, 2024

@s1na could you please PTAL at BenchmarkTransactionTrace and BenchmarkTracerStepVsCallFrame ? Not sure where/how to fix it properly

I fixed BenchmarkTransactionTrace. But re BenchmarkTracerStepVsCallFrame: works for me.

❯ go test -bench TracerStepVsCallFrame -run Foo ./core/vm/runtime
goos: darwin
goarch: arm64
pkg: github.com/ethereum/go-ethereum/core/vm/runtime
BenchmarkTracerStepVsCallFrame/tracer-step-10M-11                      3         405175861 ns/op        509118784 B/op   6363886 allocs/op
BenchmarkTracerStepVsCallFrame/tracer-call-frame-10M-11               38          30817035 ns/op           15224 B/op        192 allocs/op
PASS
ok      github.com/ethereum/go-ethereum/core/vm/runtime 4.509s

@s1na
Copy link
Contributor

s1na commented Oct 25, 2024

I noticed BenchmarkTracers/innerRevertReason and BenchmarkTracers/blobTx also failing and fixed them.

@holiman holiman force-pushed the fix_benches_2 branch 2 times, most recently from f1da70f to 207d3c4 Compare October 25, 2024 08:39
@holiman holiman marked this pull request as ready for review October 25, 2024 14:27
@holiman holiman changed the title log, triedb/pathdb: fix benchmarks all: fix issues with benchmarks Oct 25, 2024
holiman and others added 9 commits October 31, 2024 10:10
…idation-function settable

The benchmarks for assembling the pending lazy-tx lists were extremely intense in setup,

1. writing a lot of data to disk, and
2. performing a lot of expensive blob validations.

This change fixes both, by replacing billy with a no-op store, and
replacing the blobpool validation function with a yes-dummy.
@holiman holiman merged commit da17f2d into ethereum:master Nov 4, 2024
3 checks passed
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Nov 15, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Nov 15, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Nov 15, 2024
holiman added a commit that referenced this pull request Nov 19, 2024
This PR fixes some issues with benchmarks

- [x] Removes log output from a log-test
- [x] Avoids a `nil`-defer in `triedb/pathdb`
- [x] Fixes some crashes re tracers
- [x] Refactors a very resource-expensive benchmark for blobpol.
**NOTE**: this rewrite touches live production code (a little bit), as
it makes the validator-function used by the blobpool configurable.
- [x] Switch some benches over to use pebble over leveldb
- [x] reduce mem overhead in the setup-phase of some tests
- [x] Marks some tests with a long setup-phase to be skipped if `-short`
is specified (where long is on the order of tens of seconds). Ideally,
in my opinion, one should be able to run with `-benchtime 10ms -short`
and sanity-check all tests very quickly.
- [x]  Drops some metrics-bechmark which times the speed of `copy`.

---------

Co-authored-by: Sina Mahmoodi <itz.s1na@gmail.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