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

benchmark pedersen vs. sha256 hash in key computation scheme #50

Conversation

gballet
Copy link
Owner

@gballet gballet commented Jan 6, 2022

No description provided.

@gballet gballet requested a review from holiman as a code owner January 6, 2022 16:00
@gballet gballet force-pushed the verkle/get-key-pedersen-hash-benchmark branch 3 times, most recently from d390dd9 to dccba4f Compare January 6, 2022 16:02
@gballet gballet removed the request for review from holiman January 6, 2022 16:02
@gballet
Copy link
Owner Author

gballet commented Jan 6, 2022

Getting the sample data with:

go test ./trie/utils/ -run - -bench Bench --count 10 | tee pedersen.txt sha.txt

I then edit these two text files to only keep one method (pedersen or sha) and then rename the test to Hash-8.

Results of the comparison:

> benchstat sha.txt pedersen.txt
name    old time/op    new time/op       delta
Hash-8    1.27µs ±26%  359836.63µs ±82%  +28346885.22%  (p=0.000 n=10+10)

name    old alloc/op   new alloc/op      delta
Hash-8     32.0B ± 0%  27594733.9B ± 0%  +86233443.36%  (p=0.000 n=10+8)

name    old allocs/op  new allocs/op     delta
Hash-8      1.00 ± 0%    431965.33 ± 0%  +43196433.33%  (p=0.000 n=10+9)

* replace sha256 with pedersen_hash

* fix: prevent an OOB

* workaround timeout in unit test

* update go-ipa and reduce the timeout

* fix for unit tests: do not call NewAccessWitness in NewEVMTxContext (#49)

* potential fix: do not call NewAccessWitness in NewEVMTxContext

* more fixes: check for the existence of Accesses

* fix absence of witness in copy

* fix another witness issue

* workaround: ensure the prefetcher is off in verkle mode

* fix the remaining issues in tests

* review feedback

* fix witness allocation in stateless test
)

* potential fix: do not call NewAccessWitness in NewEVMTxContext

* more fixes: check for the existence of Accesses

* fix absence of witness in copy

* fix another witness issue

* workaround: ensure the prefetcher is off in verkle mode

* fix the remaining issues in tests

* review feedback

* fix witness allocation in stateless test
@gballet gballet force-pushed the verkle/get-key-pedersen-hash-benchmark branch from dccba4f to 8ca5479 Compare January 10, 2022 13:57
@gballet
Copy link
Owner Author

gballet commented Jan 10, 2022

This branch is in flux after a botched rebase, nonetheless #53 introduces the fix:

name    old time/op    new time/op     delta
Hash-8     930ns ±15%   589948ns ±12%   +63312.09%  (p=0.000 n=10+8)

name    old alloc/op   new alloc/op    delta
Hash-8     32.0B ± 0%  222662.8B ± 0%  +695721.25%  (p=0.000 n=10+10)

name    old allocs/op  new allocs/op   delta
Hash-8      1.00 ± 0%    3098.00 ± 0%  +309700.00%  (p=0.000 n=10+9)

63k-fold slower instead of 28M-fold slower. That's much better. Closing.

@gballet gballet closed this Jan 10, 2022
gballet pushed a commit that referenced this pull request Feb 14, 2023
* ethdb: use pebble

Co-authored-by: Gary Rong <[email protected]>

foo

update

* apply suggested changes

* flags: go format

node: fix ddir lookup mistake

accounts/abi/bind: fix go.mod replacement for generated binding

deps: update pebble + with fix 32-bit build

* ethdb/pebble: respect max memtable size

* core/rawdb, ethdb: enable pebble on non-32bit platforms only

* core/rawdb: fix build tags, fix some review concerns

* core/rawdb: refactor methods for database opening

* core/rawdb: remove erroneous build tag

* cmd/geth: fix the flag default handling + testcase

* cmd/geth: improve testing regarding custom backends

* ethdb/pebble, deps: update pebble dependency

* core/rawdb: replace method with Open

* ethdb/pebble: several updates for pebble (#49)

* ethdb/pebble: fix size count in batch

* ethdb/pebble: disable seek compaction

* ethdb/pebble: more fixes

* ethdb, core, cmd: polish and fixes (#50)

* cmd/utils, core/rawdb, ethdb/pebble: address some review concerns

* Update flags.go

* ethdb/pebble: minor refactors

* ethdb/pebble: avoid copy on batch replay

* ethdb: fix compilation flaw

* cmd: fix test fail due to mismatching error message

* cmd/geth, node: rename backingdb to db.engine

---------

Co-authored-by: Jared Wasinger <[email protected]>
Co-authored-by: rjl493456442 <[email protected]>
Co-authored-by: Péter Szilágyi <[email protected]>
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.

1 participant