Skip to content

perf: bench merkle stage #1497

Merged
gakonst merged 40 commits intomainfrom
perf/bench-merkle
Mar 1, 2023
Merged

perf: bench merkle stage #1497
gakonst merged 40 commits intomainfrom
perf/bench-merkle

Conversation

@joshieDo
Copy link
Collaborator

@joshieDo joshieDo commented Feb 22, 2023

Continues the work from #1324

Fixes some issues on how the trie is inserting values into the DB during its execution, and how some of the testdata is being wrongly generated.

Mainly coming from the use of tx.put::<Table>(k,v) with DUPSORT tables (such as StoragesTrie, PlainStorageState, etc...) where the value of subkey is not actually overwritten but only appended. One needs to first delete the previous entry, before upserting.

Adds reth dump-stage merkle as well.

Future improvements:

  • trie DB should take a cursor instead
  • batch storage addresses (right now its one db.tx per address)

MegaRedHand and others added 26 commits February 10, 2023 17:12
Co-authored-by: lambdaclass-user <github@lambdaclass.com>
Co-authored-by: lambdaclass-user <github@lambdaclass.com>
Co-authored-by: lambdaclass-user <github@lambdaclass.com>
Co-authored-by: lambdaclass-user <github@lambdaclass.com>
Co-authored-by: lambdaclass-user <github@lambdaclass.com>
Co-authored-by: lambdaclass-user <github@lambdaclass.com>
Co-authored-by: lambdaclass-user <github@lambdaclass.com>
Co-authored-by: lambdaclass-user <github@lambdaclass.com>
Co-authored-by: lambdaclass-user <github@lambdaclass.com>
Co-authored-by: lambdaclass-user <github@lambdaclass.com>
Co-authored-by: lambdaclass-user <github@lambdaclass.com>
Co-authored-by: lambdaclass-user <github@lambdaclass.com>
Co-authored-by: lambdaclass-user <github@lambdaclass.com>
Co-authored-by: lambdaclass-user <github@lambdaclass.com>
@joshieDo joshieDo added C-bug An unexpected or incorrect behavior A-staged-sync Related to staged sync (pipelines and stages) C-perf A change motivated by improving speed, memory usage or disk footprint labels Feb 22, 2023
@joshieDo joshieDo changed the title perf: bench merkle perf: bench merkle stage Feb 22, 2023
@joshieDo
Copy link
Collaborator Author

joshieDo commented Feb 24, 2023

Found some problematic block ranges again, so changing it to draft

@joshieDo joshieDo marked this pull request as draft February 24, 2023 06:16
@gakonst
Copy link
Member

gakonst commented Feb 24, 2023

@joshieDo where'd you land here?

@joshieDo joshieDo marked this pull request as ready for review February 27, 2023 07:10
@joshieDo
Copy link
Collaborator Author

joshieDo commented Feb 27, 2023

  • Fixed some issue on unwind during dump-stage
  • Fixed one issue with StorageHashingStage execution when going for the incremental branch. (not checking return from seek_by_key_subkey)

ci clippy failure unrelated

tried some other block ranges, and seems finally fine.

Copy link
Collaborator

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

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

Smol nits, otherwise LGTM

Copy link
Collaborator

@onbjerg onbjerg left a comment

Choose a reason for hiding this comment

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

LGTM, not sure if CI related or fixed?

@gakonst
Copy link
Member

gakonst commented Mar 1, 2023

@joshieDo apologies for this, #1568 broke your Dump Execution Stage command; do you mind fixing? you'll need to instantiate the Executor similarly to how I have changed it in the PR

@gakonst gakonst merged commit 2884eae into main Mar 1, 2023
@gakonst gakonst deleted the perf/bench-merkle branch March 1, 2023 06:20
@gakonst
Copy link
Member

gakonst commented Mar 1, 2023

Sick - ty

if let Some(value) = val {
hashed_storage.upsert(address, StorageEntry { key, value })?;
}
hashed_storage.upsert(hashed_address, StorageEntry { key, value: val })?;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hey @joshieDo is this statement correct: if the storage key is not found (Is zero) it shouldn't be found in hashed table?
I did some refactoring and this came up. .filter(|v| v.key == key) was a bug but this shouldn't be ( I added unwrap_or_default that is why it compares to U256::ZERO)
want to double check

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-staged-sync Related to staged sync (pipelines and stages) C-bug An unexpected or incorrect behavior C-perf A change motivated by improving speed, memory usage or disk footprint

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants