Skip to content

refactor: replace closure validation rules with an enum - #2395

Open
orizi wants to merge 1 commit into
claude/vm-execution-performance-32hi0cfrom
orizi/09-09-refactor_replace_closure_validation_rules_with_an_enum
Open

orizi wants to merge 1 commit into
claude/vm-execution-performance-32hi0cfrom
orizi/09-09-refactor_replace_closure_validation_rules_with_an_enum

Conversation

@orizi

@orizi orizi commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

Only two validation rules exist (range-check and signature), so the boxed-closure ValidationRule mechanism was overdesign. Replace it with an enum holding both rules; the signature rule keeps its shared signatures map directly instead of capturing it in a closure.

BREAKING: ValidationRule and Memory::add_validation_rule are now pub(crate).

Based on #2391.

🤖 Generated with Claude Code

Performance

Expected: none — this is a code-size/complexity refactor. The allocation win for range-check validation (the hot rule) already landed in #2391; the signature rule runs verify per instance, where a boxed-closure call is immeasurable.

Measured (best-of-7 vs parent #2391 branch, local; noise ±2.4%): all 12 benchmarks within noise (−3.0% to +4.7%, mixed signs, no consistent direction). As expected, no performance change.
Methodology: cairo_programs/benchmarks/*.cairo compiled with cairo-lang 0.14 --proof_mode, run via cairo-vm-cli <prog>.json --layout all_cairo --proof_mode, hyperfine best-of-7 per binary, local x86-64 Linux. Noise floor (identical-code control pair): ±2.4%.


This change is Reviewable

@orizi
orizi force-pushed the orizi/09-09-refactor_replace_closure_validation_rules_with_an_enum branch from 3a248f1 to c4f676d Compare September 9, 2026 08:39
@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown

Benchmark Results for unmodified programs 🚀

Command Mean [s] Min [s] Max [s] Relative
base big_factorial 2.215 ± 0.024 2.178 2.255 1.00
head big_factorial 2.218 ± 0.020 2.193 2.261 1.00 ± 0.01
Command Mean [s] Min [s] Max [s] Relative
base big_fibonacci 2.097 ± 0.025 2.070 2.158 1.01 ± 0.01
head big_fibonacci 2.078 ± 0.010 2.062 2.092 1.00
Command Mean [s] Min [s] Max [s] Relative
base blake2s_integration_benchmark 6.400 ± 0.042 6.357 6.475 1.00
head blake2s_integration_benchmark 6.437 ± 0.104 6.364 6.712 1.01 ± 0.02
Command Mean [s] Min [s] Max [s] Relative
base compare_arrays_200000 2.176 ± 0.020 2.150 2.223 1.00
head compare_arrays_200000 2.194 ± 0.027 2.171 2.249 1.01 ± 0.02
Command Mean [s] Min [s] Max [s] Relative
base dict_integration_benchmark 1.479 ± 0.003 1.474 1.483 1.00
head dict_integration_benchmark 1.518 ± 0.021 1.488 1.556 1.03 ± 0.01
Command Mean [s] Min [s] Max [s] Relative
base field_arithmetic_get_square_benchmark 1.133 ± 0.010 1.118 1.152 1.00 ± 0.01
head field_arithmetic_get_square_benchmark 1.128 ± 0.010 1.115 1.143 1.00
Command Mean [s] Min [s] Max [s] Relative
base integration_builtins 6.514 ± 0.149 6.403 6.876 1.01 ± 0.02
head integration_builtins 6.435 ± 0.039 6.392 6.492 1.00
Command Mean [s] Min [s] Max [s] Relative
base keccak_integration_benchmark 6.679 ± 0.069 6.606 6.824 1.00
head keccak_integration_benchmark 6.680 ± 0.113 6.579 6.972 1.00 ± 0.02
Command Mean [s] Min [s] Max [s] Relative
base linear_search 2.228 ± 0.016 2.208 2.252 1.01 ± 0.01
head linear_search 2.214 ± 0.012 2.197 2.228 1.00
Command Mean [s] Min [s] Max [s] Relative
base math_cmp_and_pow_integration_benchmark 1.644 ± 0.012 1.626 1.667 1.00
head math_cmp_and_pow_integration_benchmark 1.663 ± 0.011 1.643 1.681 1.01 ± 0.01
Command Mean [s] Min [s] Max [s] Relative
base math_integration_benchmark 1.527 ± 0.006 1.521 1.536 1.00
head math_integration_benchmark 1.545 ± 0.007 1.531 1.553 1.01 ± 0.01
Command Mean [s] Min [s] Max [s] Relative
base memory_integration_benchmark 1.303 ± 0.010 1.286 1.316 1.00
head memory_integration_benchmark 1.308 ± 0.006 1.301 1.317 1.00 ± 0.01
Command Mean [s] Min [s] Max [s] Relative
base operations_with_data_structures_benchmarks 1.570 ± 0.018 1.550 1.604 1.00
head operations_with_data_structures_benchmarks 1.580 ± 0.008 1.565 1.589 1.01 ± 0.01
Command Mean [ms] Min [ms] Max [ms] Relative
base pedersen 509.5 ± 3.4 506.3 518.5 1.00 ± 0.01
head pedersen 508.9 ± 1.8 506.1 511.2 1.00
Command Mean [ms] Min [ms] Max [ms] Relative
base poseidon_integration_benchmark 655.6 ± 2.5 653.0 661.1 1.00
head poseidon_integration_benchmark 657.9 ± 1.5 655.9 660.8 1.00 ± 0.00
Command Mean [s] Min [s] Max [s] Relative
base secp_integration_benchmark 1.725 ± 0.007 1.718 1.740 1.00
head secp_integration_benchmark 1.733 ± 0.025 1.714 1.789 1.00 ± 0.01
Command Mean [ms] Min [ms] Max [ms] Relative
base set_integration_benchmark 656.0 ± 1.6 653.6 658.2 1.00
head set_integration_benchmark 663.1 ± 4.9 655.7 669.8 1.01 ± 0.01
Command Mean [s] Min [s] Max [s] Relative
base uint256_integration_benchmark 3.961 ± 0.027 3.921 4.004 1.00
head uint256_integration_benchmark 3.965 ± 0.027 3.942 4.015 1.00 ± 0.01

@orizi
orizi force-pushed the orizi/09-09-refactor_replace_closure_validation_rules_with_an_enum branch from c4f676d to 1227f4f Compare September 9, 2026 09:47
@codecov

codecov Bot commented Sep 9, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 83.33333% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 96.23%. Comparing base (c610d41) to head (ca05e6f).

Files with missing lines Patch % Lines
vm/src/vm/runners/builtin_runner/signature.rs 76.31% 9 Missing ⚠️
Additional details and impacted files
@@                           Coverage Diff                           @@
##           claude/vm-execution-performance-32hi0c    #2395   +/-   ##
=======================================================================
  Coverage                                   96.23%   96.23%           
=======================================================================
  Files                                         107      107           
  Lines                                       37985    37978    -7     
=======================================================================
- Hits                                        36555    36549    -6     
+ Misses                                       1430     1429    -1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@orizi
orizi force-pushed the orizi/09-09-refactor_replace_closure_validation_rules_with_an_enum branch from 1227f4f to 85c0659 Compare September 9, 2026 10:06
Only two validation rules exist (range-check and signature), so the
boxed-closure ValidationRule mechanism was overdesign. Replace it with
an enum holding both rules; the signature rule keeps its shared
signatures map directly instead of capturing it in a closure.

BREAKING: ValidationRule and Memory::add_validation_rule are now
pub(crate).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@orizi
orizi force-pushed the orizi/09-09-refactor_replace_closure_validation_rules_with_an_enum branch from 85c0659 to ca05e6f Compare September 9, 2026 10:17

This branch has not been deployed

No deployments
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.

1 participant