Skip to content

Improve performance of LayeredKeyValueStorage.get method#8563

Merged
ahamlat merged 2 commits intobesu-eth:mainfrom
ahamlat:improve-layeredKeyValueStorage-get-perf
Apr 18, 2025
Merged

Improve performance of LayeredKeyValueStorage.get method#8563
ahamlat merged 2 commits intobesu-eth:mainfrom
ahamlat:improve-layeredKeyValueStorage-get-perf

Conversation

@ahamlat
Copy link
Copy Markdown
Contributor

@ahamlat ahamlat commented Apr 17, 2025

PR description

When profiling big blocks on a devnet where besu is sync'ing with lock step, I noticed in the profiling that using Bytes.toHexString for comparaison is hurting the performances because of the usage of megamorphic calls in this method, like get method on Bytes implementations. The pink part of the profiling below corresponds to the cost of the get megamorphic call

image

This PR changes the way keys are compared by working on the underlying arrays and use Arrays.compare to do the comparaison. We can see below the new profiling following this optimization.

image

Fixed Issue(s)

Thanks for sending a pull request! Have you done the following?

  • Checked out our contribution guidelines?
  • Considered documentation and added the doc-change-required label to this PR if updates are required.
  • Considered the changelog and included an update if required.
  • For database changes (e.g. KeyValueSegmentIdentifier) considered compatibility and performed forwards and backwards compatibility tests

Locally, you can run these tests to catch failures early:

  • spotless: ./gradlew spotlessApply
  • unit tests: ./gradlew build
  • acceptance tests: ./gradlew acceptanceTest
  • integration tests: ./gradlew integrationTest
  • reference tests: ./gradlew ethereum:referenceTests:referenceTests

Signed-off-by: Ameziane H. <ameziane.hamlat@consensys.net>
@ahamlat ahamlat marked this pull request as ready for review April 17, 2025 16:14
Signed-off-by: Ameziane H. <ameziane.hamlat@consensys.net>
Copy link
Copy Markdown
Contributor

@garyschulte garyschulte left a comment

Choose a reason for hiding this comment

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

Great fix. I never really liked the string comparison, but wasn't aware of the performance implications.

@ahamlat ahamlat merged commit b6390a3 into besu-eth:main Apr 18, 2025
43 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants