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

refactor(levm): simplify fill_with_zeros #2226

Merged
merged 3 commits into from
Mar 17, 2025
Merged

Conversation

Oppen
Copy link
Contributor

@Oppen Oppen commented Mar 13, 2025

Reviewing #2186 I noticed we had this helper that returned a Result for a logically impossible situation (already covered by an if just above it).
I removed that Result and also simplified the logic by just calling resize in the appropriate case.

Verified

This commit was signed with the committer’s verified signature.
Oppen Mario Rugiero
@Oppen Oppen requested a review from a team as a code owner March 13, 2025 21:16
Copy link

github-actions bot commented Mar 13, 2025

Lines of code report

Total lines added: 0
Total lines removed: 3
Total lines changed: 3

Detailed view
+------------------------------------------+-------+------+
| File                                     | Lines | Diff |
+------------------------------------------+-------+------+
| ethrex/crates/vm/levm/src/precompiles.rs | 1407  | -3   |
+------------------------------------------+-------+------+

Copy link

github-actions bot commented Mar 13, 2025

Benchmark Results Comparison

PR Results

Benchmark Results: Factorial

Command Mean [ms] Min [ms] Max [ms] Relative
revm_Factorial 234.5 ± 1.2 233.4 236.9 1.00
levm_Factorial 948.7 ± 113.8 901.3 1271.7 4.05 ± 0.49

Benchmark Results: Factorial - Recursive

Command Mean [s] Min [s] Max [s] Relative
revm_FactorialRecursive 1.371 ± 0.067 1.321 1.501 1.00
levm_FactorialRecursive 15.655 ± 0.103 15.531 15.759 11.42 ± 0.57

Benchmark Results: Fibonacci

Command Mean [ms] Min [ms] Max [ms] Relative
revm_Fibonacci 206.2 ± 12.3 197.3 240.3 1.00
levm_Fibonacci 903.2 ± 8.3 893.0 923.6 4.38 ± 0.26

Benchmark Results: ManyHashes

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ManyHashes 8.9 ± 0.1 8.7 9.0 1.00
levm_ManyHashes 18.6 ± 0.1 18.4 18.8 2.10 ± 0.03

Benchmark Results: BubbleSort

Command Mean [s] Min [s] Max [s] Relative
revm_BubbleSort 3.218 ± 0.012 3.203 3.237 1.00
levm_BubbleSort 6.068 ± 0.035 6.035 6.126 1.89 ± 0.01

Benchmark Results: ERC20 - Transfer

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ERC20Transfer 248.1 ± 2.3 245.3 251.6 1.00
levm_ERC20Transfer 538.7 ± 1.8 536.7 541.9 2.17 ± 0.02

Benchmark Results: ERC20 - Mint

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ERC20Mint 139.8 ± 0.9 138.7 141.4 1.00
levm_ERC20Mint 351.2 ± 1.8 348.8 355.0 2.51 ± 0.02

Benchmark Results: ERC20 - Approval

Command Mean [s] Min [s] Max [s] Relative
revm_ERC20Approval 1.038 ± 0.007 1.029 1.048 1.00
levm_ERC20Approval 2.020 ± 0.017 2.007 2.066 1.95 ± 0.02

Main Results

Benchmark Results: Factorial

Command Mean [ms] Min [ms] Max [ms] Relative
revm_Factorial 234.7 ± 0.5 234.1 235.7 1.00
levm_Factorial 914.2 ± 8.7 904.3 927.7 3.90 ± 0.04

Benchmark Results: Factorial - Recursive

Command Mean [s] Min [s] Max [s] Relative
revm_FactorialRecursive 1.411 ± 0.096 1.313 1.550 1.00
levm_FactorialRecursive 15.667 ± 0.108 15.536 15.832 11.10 ± 0.76

Benchmark Results: Fibonacci

Command Mean [ms] Min [ms] Max [ms] Relative
revm_Fibonacci 202.7 ± 0.7 202.2 204.7 1.00
levm_Fibonacci 905.3 ± 8.4 897.6 920.4 4.47 ± 0.04

Benchmark Results: ManyHashes

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ManyHashes 8.6 ± 0.1 8.6 8.8 1.00
levm_ManyHashes 18.3 ± 0.1 18.1 18.5 2.12 ± 0.02

Benchmark Results: BubbleSort

Command Mean [s] Min [s] Max [s] Relative
revm_BubbleSort 3.241 ± 0.040 3.219 3.351 1.00
levm_BubbleSort 6.087 ± 0.028 6.055 6.152 1.88 ± 0.02

Benchmark Results: ERC20 - Transfer

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ERC20Transfer 249.7 ± 3.5 246.9 257.9 1.00
levm_ERC20Transfer 541.2 ± 3.4 537.3 547.1 2.17 ± 0.03

Benchmark Results: ERC20 - Mint

Command Mean [ms] Min [ms] Max [ms] Relative
revm_ERC20Mint 139.2 ± 1.0 138.0 141.5 1.00
levm_ERC20Mint 355.1 ± 2.5 351.1 359.1 2.55 ± 0.03

Benchmark Results: ERC20 - Approval

Command Mean [s] Min [s] Max [s] Relative
revm_ERC20Approval 1.041 ± 0.011 1.030 1.062 1.00
levm_ERC20Approval 2.024 ± 0.013 2.006 2.051 1.95 ± 0.02

@fborello-lambda fborello-lambda changed the title refactor: simplify fill_with_zeros refactor(levm): simplify fill_with_zeros Mar 14, 2025
Copy link
Contributor

@fborello-lambda fborello-lambda left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link
Contributor

@LeanSerra LeanSerra left a comment

Choose a reason for hiding this comment

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

LGTM

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@ilitteri ilitteri enabled auto-merge March 14, 2025 20:05

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@ilitteri ilitteri added this pull request to the merge queue Mar 17, 2025
Merged via the queue into main with commit 5844371 Mar 17, 2025
28 checks passed
@ilitteri ilitteri deleted the refactor/cleaner_padding branch March 17, 2025 19:08
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

5 participants