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

Optimized Keccak Hashing for Account Storage Slots #6452

Merged
merged 22 commits into from Jan 30, 2024

Conversation

matkt
Copy link
Contributor

@matkt matkt commented Jan 23, 2024

PR description

The purpose of this PR is to avoid unnecessary recomputations of the Keccak hash for account storage slots during the commit phase. In most cases, we have already calculated the hashes during the transaction processing phase and can reuse these values. The idea is to add this information to the accumulator, storing each key-to-hash mapping as it's generated during the transaction. At the end of the block, during the commit phase, we will first check the cache before attempting to recalculate the hash.

The cache size should not pose a problem as it will inevitably contain fewer elements than the other maps in the accumulator. This PR also has an added benefit in cases where we need to access the same slot key for two different accounts during the transaction processing phase. In such cases, the hash will only be calculated once.

Fixed Issue(s)

Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Copy link

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.
  • I thought about the changelog and included a changelog update if required.
  • If my PR includes database changes (e.g. KeyValueSegmentIdentifier) I have thought about compatibility and performed forwards and backwards compatibility tests

ahamlat and others added 12 commits January 25, 2024 15:42
Signed-off-by: Ameziane H <ameziane.hamlat@consensys.net>
Signed-off-by: Ameziane H <ameziane.hamlat@consensys.net>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: ahamlat <ameziane.hamlat@consensys.net>
Signed-off-by: Ameziane H <ameziane.hamlat@consensys.net>
ahamlat and others added 5 commits January 30, 2024 14:14
Signed-off-by: Ameziane H <ameziane.hamlat@consensys.net>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
@ahamlat
Copy link
Contributor

ahamlat commented Jan 30, 2024

Regarding performance, there is a clear improvement on block processing with this PR as we can see in the screenshot below

image

Examining the CPU profiling also reveals the improvement in the Commit phase of block processing.

Before this PR

image

With this PR

image

@matkt matkt changed the title Feature/keccak impro Optimized Keccak Hashing for Account Storage Slots Jan 30, 2024
@matkt matkt marked this pull request as ready for review January 30, 2024 13:55
Signed-off-by: Karim Taam <karim.t2am@gmail.com>
@matkt matkt changed the title Optimized Keccak Hashing for Account Storage Slots DO NOT MERGE : Optimized Keccak Hashing for Account Storage Slots Jan 30, 2024
@garyschulte garyschulte changed the title DO NOT MERGE : Optimized Keccak Hashing for Account Storage Slots Optimized Keccak Hashing for Account Storage Slots Jan 30, 2024
@garyschulte
Copy link
Contributor

changed to Mergeable after @ahamlat confirmed testing regressions were unrelated to this PR

Signed-off-by: garyschulte <garyschulte@gmail.com>
@garyschulte garyschulte enabled auto-merge (squash) January 30, 2024 19:27
@garyschulte garyschulte merged commit 6d77d58 into hyperledger:main Jan 30, 2024
18 checks passed
jflo pushed a commit to jflo/besu that referenced this pull request Jan 30, 2024
* not recompute hash if not needed
* Add memoize for the Supplier
* Modify hashcode to only process keccak when slotkey is not defined
* use single cache for keccak

Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Ameziane H <ameziane.hamlat@consensys.net>
Signed-off-by: ahamlat <ameziane.hamlat@consensys.net>
Signed-off-by: garyschulte <garyschulte@gmail.com>
jflo pushed a commit to jflo/besu that referenced this pull request Jan 30, 2024
* not recompute hash if not needed
* Add memoize for the Supplier
* Modify hashcode to only process keccak when slotkey is not defined
* use single cache for keccak

Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Ameziane H <ameziane.hamlat@consensys.net>
Signed-off-by: ahamlat <ameziane.hamlat@consensys.net>
Signed-off-by: garyschulte <garyschulte@gmail.com>
jflo pushed a commit to jflo/besu that referenced this pull request Jan 30, 2024
* not recompute hash if not needed
* Add memoize for the Supplier
* Modify hashcode to only process keccak when slotkey is not defined
* use single cache for keccak

Signed-off-by: Karim Taam <karim.t2am@gmail.com>
Signed-off-by: Ameziane H <ameziane.hamlat@consensys.net>
Signed-off-by: ahamlat <ameziane.hamlat@consensys.net>
Signed-off-by: garyschulte <garyschulte@gmail.com>
Signed-off-by: Justin Florentine <justin+github@florentine.us>
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

3 participants