Skip to content

Route ModExp precompile in certain conditions #8868

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Jun 28, 2025
Merged

Route ModExp precompile in certain conditions #8868

merged 3 commits into from
Jun 28, 2025

Conversation

lu-pinto
Copy link
Member

@lu-pinto lu-pinto commented Jun 27, 2025 •
edited
Loading

PR description

This is a cherry-pick from the performance branch that we used to test the changes in the latest interop.

original PR: #8758

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

@ahamlat
Copy link
Contributor

ahamlat commented Jun 27, 2025

@lu-pinto I think this commit 9ece56b is missing

@lu-pinto
Copy link
Member Author

results of the benchmarks attached, with improvements all across the board:
image

siladu
siladu approved these changes Jun 28, 2025
View reviewed changes
Copy link
Contributor

@siladu siladu left a comment •
edited
Loading

Choose a reason for hiding this comment

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

results of the benchmarks attached, with improvements all across the board:

It's curious that even the cases with no optimisation triggered are better than before

.../org/hyperledger/besu/evm/precompile/BigIntegerModularExponentiationPrecompiledContract.java Outdated
final int modulusLength = clampedToInt(length_of_MODULUS);
if ((extractLastByte(input, baseOffset, baseLength) & 1) != 1
&& (extractLastByte(input, modulusOffset, modulusLength) & 1) != 1) {
return computeNative(input, length_of_MODULUS);
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe could pass int modulusLength instead of recalculating?

@lu-pinto lu-pinto enabled auto-merge (squash) June 28, 2025 09:38
@lu-pinto lu-pinto force-pushed the route-modexp-precompile branch from 1169432 to a38cd40 Compare June 28, 2025 09:41
lu-pinto and others added 3 commits June 28, 2025 10:46
@lu-pinto
Route ModExp precompile in certain conditions
6f16108 
Signed-off-by: Luis Pinto <luis.pinto@consensys.net>
@ahamlat @lu-pinto
Disable native on ModExp
5e1ff6d 
Signed-off-by: Ameziane H. <ameziane.hamlat@consensys.net>
@lu-pinto
pass clamped int to precompile computation
74b580a 
Signed-off-by: Luis Pinto <luis.pinto@consensys.net>
@lu-pinto lu-pinto force-pushed the route-modexp-precompile branch from a38cd40 to 74b580a Compare June 28, 2025 09:46
@lu-pinto lu-pinto merged commit 9915379 into hyperledger:main Jun 28, 2025
58 of 63 checks passed
@macfarla macfarla mentioned this pull request Jun 30, 2025
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@siladu siladu siladu approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lu-pinto @ahamlat @siladu