Skip to content

Conversation

@fab-10
Copy link
Contributor

@fab-10 fab-10 commented Nov 14, 2025

PR description

I found that there was an unnecessary array created on every call to TransactionType::of so I decided to do a small PR to remove that allocation, since the method should be called many times, but then I also found a bunch of other small optimizations that ended up in this PR, where all the static values are pre-calculated and set at the beginning, to avoid set lookup and comparison.

In a micro benchmark the new version of TransactionType::of (renamed to TransactionType::fromOpaque is now 20x faster

Before

Benchmark                                  Mode  Cnt    Score   Error  Units
TransactionTypeBenchmark.ofSerializedType  avgt   15  120.077 ± 9.074  ns/op

After

Benchmark                                  Mode  Cnt  Score   Error  Units
TransactionTypeBenchmark.ofSerializedType  avgt   15  5.857 ± 0.266  ns/op

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
  • hive tests: Engine or other RPCs modified?

@fab-10 fab-10 force-pushed the tx-type-optimizations branch 7 times, most recently from 210eae6 to 2e27a51 Compare November 14, 2025 17:59
@fab-10 fab-10 force-pushed the tx-type-optimizations branch 10 times, most recently from bd1d253 to d91b049 Compare November 17, 2025 21:33
@fab-10 fab-10 force-pushed the tx-type-optimizations branch from d91b049 to b062368 Compare November 18, 2025 09:42
@fab-10 fab-10 marked this pull request as ready for review November 20, 2025 15:51
Copy link
Contributor

@macfarla macfarla left a comment

Choose a reason for hiding this comment

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

LGTM

@fab-10 fab-10 enabled auto-merge (squash) November 24, 2025 09:37
@fab-10 fab-10 merged commit ee2cdd5 into hyperledger:main Nov 24, 2025
46 checks passed
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