fix: remove feature mod_builtin by OmriEshhar1 · Pull Request #2387 · starkware-libs/cairo-vm

OmriEshhar1 · 2026-06-03T07:54:30Z

TITLE

Description

Description of the pull request changes and motivation.

Checklist

Linked to Github Issue
Unit tests added
Integration tests added.
This change requires new documentation.
- Documentation has been added/updated.
- CHANGELOG has been updated.

This change is

github-actions · 2026-06-03T08:10:52Z

**Hyper Thereading Benchmark results**




hyperfine -r 2 -n "hyper_threading_main threads: 1" 'RAYON_NUM_THREADS=1 ./hyper_threading_main' -n "hyper_threading_pr threads: 1" 'RAYON_NUM_THREADS=1 ./hyper_threading_pr'
Benchmark 1: hyper_threading_main threads: 1
  Time (mean ± σ):     22.263 s ±  0.002 s    [User: 21.694 s, System: 0.566 s]
  Range (min … max):   22.262 s … 22.264 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 1
  Time (mean ± σ):     22.523 s ±  0.037 s    [User: 21.962 s, System: 0.558 s]
  Range (min … max):   22.497 s … 22.549 s    2 runs
 
Summary
  hyper_threading_main threads: 1 ran
    1.01 ± 0.00 times faster than hyper_threading_pr threads: 1




hyperfine -r 2 -n "hyper_threading_main threads: 2" 'RAYON_NUM_THREADS=2 ./hyper_threading_main' -n "hyper_threading_pr threads: 2" 'RAYON_NUM_THREADS=2 ./hyper_threading_pr'
Benchmark 1: hyper_threading_main threads: 2
  Time (mean ± σ):     11.943 s ±  0.002 s    [User: 21.747 s, System: 0.598 s]
  Range (min … max):   11.941 s … 11.944 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 2
  Time (mean ± σ):     12.093 s ±  0.008 s    [User: 22.113 s, System: 0.601 s]
  Range (min … max):   12.087 s … 12.099 s    2 runs
 
Summary
  hyper_threading_main threads: 2 ran
    1.01 ± 0.00 times faster than hyper_threading_pr threads: 2




hyperfine -r 2 -n "hyper_threading_main threads: 4" 'RAYON_NUM_THREADS=4 ./hyper_threading_main' -n "hyper_threading_pr threads: 4" 'RAYON_NUM_THREADS=4 ./hyper_threading_pr'
Benchmark 1: hyper_threading_main threads: 4
  Time (mean ± σ):      9.777 s ±  0.022 s    [User: 34.892 s, System: 0.691 s]
  Range (min … max):    9.761 s …  9.792 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 4
  Time (mean ± σ):      9.786 s ±  0.322 s    [User: 35.259 s, System: 0.744 s]
  Range (min … max):    9.558 s … 10.013 s    2 runs
 
Summary
  hyper_threading_main threads: 4 ran
    1.00 ± 0.03 times faster than hyper_threading_pr threads: 4




hyperfine -r 2 -n "hyper_threading_main threads: 6" 'RAYON_NUM_THREADS=6 ./hyper_threading_main' -n "hyper_threading_pr threads: 6" 'RAYON_NUM_THREADS=6 ./hyper_threading_pr'
Benchmark 1: hyper_threading_main threads: 6
  Time (mean ± σ):      9.540 s ±  0.127 s    [User: 35.906 s, System: 0.748 s]
  Range (min … max):    9.450 s …  9.630 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 6
  Time (mean ± σ):      9.576 s ±  0.048 s    [User: 35.415 s, System: 0.732 s]
  Range (min … max):    9.542 s …  9.610 s    2 runs
 
Summary
  hyper_threading_main threads: 6 ran
    1.00 ± 0.01 times faster than hyper_threading_pr threads: 6




hyperfine -r 2 -n "hyper_threading_main threads: 8" 'RAYON_NUM_THREADS=8 ./hyper_threading_main' -n "hyper_threading_pr threads: 8" 'RAYON_NUM_THREADS=8 ./hyper_threading_pr'
Benchmark 1: hyper_threading_main threads: 8
  Time (mean ± σ):      9.523 s ±  0.045 s    [User: 36.059 s, System: 0.757 s]
  Range (min … max):    9.491 s …  9.554 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 8
  Time (mean ± σ):      9.588 s ±  0.079 s    [User: 35.543 s, System: 0.764 s]
  Range (min … max):    9.532 s …  9.644 s    2 runs
 
Summary
  hyper_threading_main threads: 8 ran
    1.01 ± 0.01 times faster than hyper_threading_pr threads: 8




hyperfine -r 2 -n "hyper_threading_main threads: 16" 'RAYON_NUM_THREADS=16 ./hyper_threading_main' -n "hyper_threading_pr threads: 16" 'RAYON_NUM_THREADS=16 ./hyper_threading_pr'
Benchmark 1: hyper_threading_main threads: 16
  Time (mean ± σ):      9.707 s ±  0.246 s    [User: 36.231 s, System: 0.802 s]
  Range (min … max):    9.534 s …  9.881 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 16
  Time (mean ± σ):      9.810 s ±  0.157 s    [User: 35.711 s, System: 0.835 s]
  Range (min … max):    9.698 s …  9.921 s    2 runs
 
Summary
  hyper_threading_main threads: 16 ran
    1.01 ± 0.03 times faster than hyper_threading_pr threads: 16

codecov · 2026-06-03T08:45:23Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.21%. Comparing base (9528e66) to head (03af660).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2387      +/-   ##
==========================================
+ Coverage   96.20%   96.21%   +0.01%     
==========================================
  Files         107      107              
  Lines       37963    37925      -38     
==========================================
- Hits        36522    36490      -32     
+ Misses       1441     1435       -6

☔ 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.

github-actions · 2026-06-03T08:47:43Z

Benchmark Results for unmodified programs 🚀

Command	Mean [s]	Min [s]	Max [s]	Relative
`base big_factorial`	2.100 ± 0.007	2.094	2.115	1.00
`head big_factorial`	2.107 ± 0.014	2.092	2.146	1.00 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
`base big_fibonacci`	2.042 ± 0.054	2.019	2.194	1.01 ± 0.03
`head big_fibonacci`	2.032 ± 0.010	2.022	2.051	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base blake2s_integration_benchmark`	7.207 ± 0.200	7.038	7.710	1.00 ± 0.04
`head blake2s_integration_benchmark`	7.195 ± 0.164	7.096	7.623	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base compare_arrays_200000`	2.170 ± 0.008	2.158	2.185	1.00 ± 0.01
`head compare_arrays_200000`	2.165 ± 0.010	2.150	2.179	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base dict_integration_benchmark`	1.428 ± 0.004	1.422	1.438	1.00 ± 0.01
`head dict_integration_benchmark`	1.424 ± 0.007	1.419	1.442	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base field_arithmetic_get_square_benchmark`	1.204 ± 0.008	1.198	1.227	1.00 ± 0.01
`head field_arithmetic_get_square_benchmark`	1.203 ± 0.008	1.195	1.224	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base integration_builtins`	7.178 ± 0.016	7.148	7.197	1.00
`head integration_builtins`	7.274 ± 0.064	7.216	7.441	1.01 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
`base keccak_integration_benchmark`	7.299 ± 0.046	7.253	7.369	1.00 ± 0.01
`head keccak_integration_benchmark`	7.286 ± 0.061	7.193	7.388	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base linear_search`	2.128 ± 0.004	2.121	2.133	1.00
`head linear_search`	2.166 ± 0.024	2.136	2.203	1.02 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
`base math_cmp_and_pow_integration_benchmark`	1.512 ± 0.021	1.498	1.569	1.01 ± 0.02
`head math_cmp_and_pow_integration_benchmark`	1.502 ± 0.023	1.485	1.551	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base math_integration_benchmark`	1.471 ± 0.007	1.461	1.484	1.01 ± 0.01
`head math_integration_benchmark`	1.453 ± 0.007	1.446	1.469	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base memory_integration_benchmark`	1.207 ± 0.006	1.201	1.218	1.00
`head memory_integration_benchmark`	1.217 ± 0.020	1.204	1.271	1.01 ± 0.02

Command	Mean [s]	Min [s]	Max [s]	Relative
`base operations_with_data_structures_benchmarks`	1.550 ± 0.009	1.540	1.570	1.01 ± 0.01
`head operations_with_data_structures_benchmarks`	1.530 ± 0.004	1.524	1.535	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base pedersen`	529.6 ± 3.6	524.9	537.8	1.00 ± 0.01
`head pedersen`	528.1 ± 4.3	525.2	540.2	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base poseidon_integration_benchmark`	614.4 ± 9.9	605.5	633.2	1.00
`head poseidon_integration_benchmark`	614.5 ± 5.1	608.9	626.3	1.00 ± 0.02

Command	Mean [s]	Min [s]	Max [s]	Relative
`base secp_integration_benchmark`	1.801 ± 0.006	1.793	1.810	1.00 ± 0.01
`head secp_integration_benchmark`	1.799 ± 0.011	1.789	1.828	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base set_integration_benchmark`	672.9 ± 1.9	669.3	676.0	1.00
`head set_integration_benchmark`	681.6 ± 6.1	678.1	698.7	1.01 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
`base uint256_integration_benchmark`	4.148 ± 0.060	4.098	4.259	1.01 ± 0.02
`head uint256_integration_benchmark`	4.125 ± 0.034	4.103	4.215	1.00

noam-starkware

@noam-starkware made 3 comments.
Reviewable status: 0 of 9 files reviewed, 3 unresolved discussions (waiting on OmriEshhar1).

a discussion (no related file):
Can you delete the definition of the feature itself and not just its invocations? It's defined several times in multiple Cargo.toml files.
Once deleted, I think it would require some more changes (for example, the cairo1 makefile uses this feature flag for running targets, and I think that in rust.yml as well).

a discussion (no related file):
The cairo1-run/README.md and onboarding.md files mention mod_builtin feature. Should these refs be removed?

vm/src/vm/runners/cairo_runner.rs line 5884 at r1 (raw file):

        ];
        // Match the all_cairo_stwo layout builtins (mod builtins excluded
        // unless the mod_builtin feature is enabled).

delete parentheses.

Code quote:

        // Match the all_cairo_stwo layout builtins (mod builtins excluded
        // unless the mod_builtin feature is enabled).

OmriEshhar1

@OmriEshhar1 made 3 comments.
Reviewable status: 0 of 16 files reviewed, 3 unresolved discussions (waiting on noam-starkware and OmriEshhar1).

a discussion (no related file):

Previously, noam-starkware wrote…

The cairo1-run/README.md and onboarding.md files mention mod_builtin feature. Should these refs be removed?

Done.

a discussion (no related file):

Previously, noam-starkware wrote…

Can you delete the definition of the feature itself and not just its invocations? It's defined several times in multiple Cargo.toml files.
Once deleted, I think it would require some more changes (for example, the cairo1 makefile uses this feature flag for running targets, and I think that in rust.yml as well).

Done.

vm/src/vm/runners/cairo_runner.rs line 5884 at r1 (raw file):

Previously, noam-starkware wrote…

delete parentheses.

Done.

noam-starkware

@noam-starkware reviewed 16 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on OmriEshhar1).

a discussion (no related file):

Previously, OmriEshhar1 wrote…

Done.

One definition of the feature remains in vm/Cargo.toml.

Also, in the .github/workflows/rust.yml files you removed the feature definition but I think that the action itself still refers to it.

I asked Claude about it and this is the response I got:

  What upload-coverage does:

  It collects the lcov-*.info coverage files produced by the tests job (which runs 4×4=16 matrix combinations of 4 partitions × 4 special_features values), fetches them from
  the GitHub Actions cache, and uploads them all to codecov.io in one shot.

  The problem — there's a mismatch right now:

  The tests job matrix (line 297) lists these special_features:
  special_features: ["", "extensive_hints", "cairo-0-secp-hints", "cairo-0-data-availability-hints"]
  
  But upload-coverage (lines 545–568) still tries to fetch 4 files for mod_builtin:
  - name: Fetch results for tests with stdlib (w/mod_builtin; part. 1)
    uses: actions/cache/restore@v3
    with: 
      path: lcov-test#1-mod_builtin.info
      key: codecov-cache-test#1-mod_builtin-${{ github.sha }}
      fail-on-cache-miss: true   # <-- this will FAIL
      
  Since mod_builtin is no longer in the tests matrix, those cache entries will never be written, and fail-on-cache-miss: true means the upload-coverage job will hard-fail.

  Additionally, cairo-0-data-availability-hints is in the tests matrix but has no corresponding fetch steps in upload-coverage, so its coverage would be silently dropped.

  What needs to change:

  Remove the 4 mod_builtin fetch steps (lines 545–568) and add 4 equivalent steps for cairo-0-data-availability-hints. The pattern is identical — just swap the feature name:

  - name: Fetch results for tests with stdlib (w/cairo-0-data-availability-hints; part. 1)
    uses: actions/cache/restore@v3
    with:
      path: lcov-test#1-cairo-0-data-availability-hints.info
      key: codecov-cache-test#1-cairo-0-data-availability-hints-${{ github.sha }}
      fail-on-cache-miss: true
  # ... repeat for parts 2, 3, 4

OmriEshhar1

@OmriEshhar1 made 1 comment.
Reviewable status: 15 of 17 files reviewed, 2 unresolved discussions (waiting on noam-starkware and OmriEshhar1).

a discussion (no related file):

Previously, noam-starkware wrote…

One definition of the feature remains in vm/Cargo.toml.

Also, in the .github/workflows/rust.yml files you removed the feature definition but I think that the action itself still refers to it.

I asked Claude about it and this is the response I got:

  What upload-coverage does:

  It collects the lcov-*.info coverage files produced by the tests job (which runs 4×4=16 matrix combinations of 4 partitions × 4 special_features values), fetches them from
  the GitHub Actions cache, and uploads them all to codecov.io in one shot.

  The problem — there's a mismatch right now:

  The tests job matrix (line 297) lists these special_features:
  special_features: ["", "extensive_hints", "cairo-0-secp-hints", "cairo-0-data-availability-hints"]
  
  But upload-coverage (lines 545–568) still tries to fetch 4 files for mod_builtin:
  - name: Fetch results for tests with stdlib (w/mod_builtin; part. 1)
    uses: actions/cache/restore@v3
    with: 
      path: lcov-test#1-mod_builtin.info
      key: codecov-cache-test#1-mod_builtin-${{ github.sha }}
      fail-on-cache-miss: true   # <-- this will FAIL
      
  Since mod_builtin is no longer in the tests matrix, those cache entries will never be written, and fail-on-cache-miss: true means the upload-coverage job will hard-fail.

  Additionally, cairo-0-data-availability-hints is in the tests matrix but has no corresponding fetch steps in upload-coverage, so its coverage would be silently dropped.

  What needs to change:

  Remove the 4 mod_builtin fetch steps (lines 545–568) and add 4 equivalent steps for cairo-0-data-availability-hints. The pattern is identical — just swap the feature name:

  - name: Fetch results for tests with stdlib (w/cairo-0-data-availability-hints; part. 1)
    uses: actions/cache/restore@v3
    with:
      path: lcov-test#1-cairo-0-data-availability-hints.info
      key: codecov-cache-test#1-cairo-0-data-availability-hints-${{ github.sha }}
      fail-on-cache-miss: true
  # ... repeat for parts 2, 3, 4

Done.

OmriEshhar1

@OmriEshhar1 reviewed 2 files and all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on noam-starkware).

OmriEshhar1

@OmriEshhar1 partially reviewed 15 files.
Reviewable status: 13 of 32 files reviewed, 2 unresolved discussions (waiting on noam-starkware).

OmriEshhar1

@OmriEshhar1 reviewed 15 files.
Reviewable status: 28 of 32 files reviewed, 2 unresolved discussions (waiting on noam-starkware).

OmriEshhar1

@OmriEshhar1 reviewed 16 files and all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on noam-starkware).

OmriEshhar1

@OmriEshhar1 reviewed all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on noam-starkware).

noam-starkware

@noam-starkware reviewed 20 files and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on OmriEshhar1).

OmriEshhar1 self-assigned this Jun 3, 2026

OmriEshhar1 changed the title ~~Remove feature mod_builtin~~ fix: remove feature mod_builtin Jun 3, 2026

OmriEshhar1 force-pushed the omri/remove-mod_builtin-feature branch from 764d94b to 901171a Compare June 3, 2026 08:04

OmriEshhar1 force-pushed the omri/remove-mod_builtin-feature branch 2 times, most recently from 84f5b1b to 2106d48 Compare June 3, 2026 08:29

noam-starkware requested changes Jun 3, 2026

View reviewed changes

OmriEshhar1 force-pushed the omri/remove-mod_builtin-feature branch from 2106d48 to a274beb Compare June 3, 2026 12:31

OmriEshhar1 commented Jun 3, 2026

View reviewed changes

noam-starkware requested changes Jun 7, 2026

View reviewed changes

Remove feature mod_builtin

32345a1

OmriEshhar1 force-pushed the omri/remove-mod_builtin-feature branch from a274beb to 32345a1 Compare June 7, 2026 07:27

OmriEshhar1 commented Jun 7, 2026

View reviewed changes

OmriEshhar1 force-pushed the omri/remove-mod_builtin-feature branch from 8aff3c8 to 89b4c33 Compare June 7, 2026 10:28

OmriEshhar1 commented Jun 7, 2026

View reviewed changes

OmriEshhar1 force-pushed the omri/remove-mod_builtin-feature branch from 89b4c33 to 7e60269 Compare June 7, 2026 12:00

OmriEshhar1 commented Jun 7, 2026

View reviewed changes

OmriEshhar1 force-pushed the omri/remove-mod_builtin-feature branch from 7e60269 to d22415c Compare June 7, 2026 12:19

change CI branch

03af660

OmriEshhar1 force-pushed the omri/remove-mod_builtin-feature branch from d22415c to 03af660 Compare June 7, 2026 12:24

OmriEshhar1 commented Jun 7, 2026

View reviewed changes

noam-starkware approved these changes Jun 7, 2026

View reviewed changes

OmriEshhar1 added this pull request to the merge queue Jun 7, 2026

Merged via the queue into main with commit d26a502 Jun 7, 2026
64 checks passed

OmriEshhar1 deleted the omri/remove-mod_builtin-feature branch June 7, 2026 13:12

Conversation

OmriEshhar1 commented Jun 3, 2026 • edited by phil-starkware Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

TITLE

Description

Checklist

Uh oh!

github-actions Bot commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Jun 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

noam-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

OmriEshhar1 left a comment

Choose a reason for hiding this comment

Uh oh!

noam-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

OmriEshhar1 left a comment

Choose a reason for hiding this comment

Uh oh!

OmriEshhar1 left a comment

Choose a reason for hiding this comment

Uh oh!

OmriEshhar1 left a comment

Choose a reason for hiding this comment

Uh oh!

OmriEshhar1 left a comment

Choose a reason for hiding this comment

Uh oh!

OmriEshhar1 left a comment

Choose a reason for hiding this comment

Uh oh!

OmriEshhar1 left a comment

Choose a reason for hiding this comment

Uh oh!

noam-starkware left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

OmriEshhar1 commented Jun 3, 2026 •

edited by phil-starkware

Loading

github-actions Bot commented Jun 3, 2026 •

edited

Loading

codecov Bot commented Jun 3, 2026 •

edited

Loading

github-actions Bot commented Jun 3, 2026 •

edited

Loading