Limited padding of segments to >=16

[yuvalsw]

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]

**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 ± σ):     26.579 s ±  0.013 s    [User: 25.660 s, System: 0.917 s]
  Range (min … max):   26.570 s … 26.588 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 1
  Time (mean ± σ):     26.677 s ±  0.021 s    [User: 25.734 s, System: 0.942 s]
  Range (min … max):   26.662 s … 26.692 s    2 runs
 
Summary
  hyper_threading_main threads: 1 ran
    1.00 ± 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 ± σ):     14.756 s ±  0.007 s    [User: 25.725 s, System: 0.932 s]
  Range (min … max):   14.751 s … 14.761 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 2
  Time (mean ± σ):     14.840 s ±  0.003 s    [User: 25.859 s, System: 0.957 s]
  Range (min … max):   14.838 s … 14.842 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 ± σ):     10.694 s ±  0.318 s    [User: 38.055 s, System: 1.076 s]
  Range (min … max):   10.469 s … 10.919 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 4
  Time (mean ± σ):     10.529 s ±  0.194 s    [User: 38.182 s, System: 1.070 s]
  Range (min … max):   10.392 s … 10.665 s    2 runs
 
Summary
  hyper_threading_pr threads: 4 ran
    1.02 ± 0.04 times faster than hyper_threading_main 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 ± σ):     10.591 s ±  0.172 s    [User: 38.435 s, System: 1.097 s]
  Range (min … max):   10.470 s … 10.713 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 6
  Time (mean ± σ):     10.582 s ±  0.214 s    [User: 38.329 s, System: 1.117 s]
  Range (min … max):   10.431 s … 10.734 s    2 runs
 
Summary
  hyper_threading_pr threads: 6 ran
    1.00 ± 0.03 times faster than hyper_threading_main 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 ± σ):     10.449 s ±  0.122 s    [User: 38.778 s, System: 1.160 s]
  Range (min … max):   10.363 s … 10.535 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 8
  Time (mean ± σ):     10.524 s ±  0.058 s    [User: 38.943 s, System: 1.130 s]
  Range (min … max):   10.483 s … 10.565 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 ± σ):     10.736 s ±  0.004 s    [User: 38.911 s, System: 1.204 s]
  Range (min … max):   10.734 s … 10.739 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 16
  Time (mean ± σ):     10.543 s ±  0.342 s    [User: 39.249 s, System: 1.196 s]
  Range (min … max):   10.302 s … 10.785 s    2 runs
 
Summary
  hyper_threading_pr threads: 16 ran
    1.02 ± 0.03 times faster than hyper_threading_main threads: 16

[github-actions]

Benchmark Results for unmodified programs 🚀

Command	Mean [s]	Min [s]	Max [s]	Relative
base big_factorial	2.153 ± 0.013	2.138	2.175	1.00 ± 0.01
head big_factorial	2.146 ± 0.025	2.123	2.209	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
base big_fibonacci	2.085 ± 0.017	2.065	2.121	1.00
head big_fibonacci	2.090 ± 0.022	2.068	2.124	1.00 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
base blake2s_integration_benchmark	7.847 ± 0.116	7.706	8.095	1.02 ± 0.02
head blake2s_integration_benchmark	7.685 ± 0.139	7.552	7.946	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
base compare_arrays_200000	2.197 ± 0.017	2.171	2.219	1.00
head compare_arrays_200000	2.202 ± 0.023	2.177	2.253	1.00 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
base dict_integration_benchmark	1.427 ± 0.012	1.416	1.457	1.00
head dict_integration_benchmark	1.444 ± 0.007	1.429	1.455	1.01 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
base field_arithmetic_get_square_benchmark	1.239 ± 0.012	1.222	1.267	1.01 ± 0.02
head field_arithmetic_get_square_benchmark	1.227 ± 0.021	1.204	1.263	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
base integration_builtins	7.905 ± 0.100	7.784	8.157	1.03 ± 0.01
head integration_builtins	7.710 ± 0.051	7.637	7.780	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
base keccak_integration_benchmark	8.090 ± 0.063	8.003	8.175	1.01 ± 0.02
head keccak_integration_benchmark	8.045 ± 0.162	7.885	8.342	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
base linear_search	2.188 ± 0.019	2.157	2.227	1.00 ± 0.01
head linear_search	2.181 ± 0.021	2.155	2.214	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
base math_cmp_and_pow_integration_benchmark	1.511 ± 0.010	1.495	1.525	1.00
head math_cmp_and_pow_integration_benchmark	1.525 ± 0.016	1.514	1.563	1.01 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
base math_integration_benchmark	1.466 ± 0.012	1.451	1.499	1.00 ± 0.01
head math_integration_benchmark	1.464 ± 0.011	1.448	1.489	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
base memory_integration_benchmark	1.220 ± 0.010	1.208	1.233	1.00
head memory_integration_benchmark	1.233 ± 0.011	1.213	1.246	1.01 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
base operations_with_data_structures_benchmarks	1.583 ± 0.008	1.571	1.597	1.00 ± 0.01
head operations_with_data_structures_benchmarks	1.577 ± 0.012	1.566	1.606	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
base pedersen	538.0 ± 2.6	533.4	542.4	1.00 ± 0.01
head pedersen	536.3 ± 3.1	532.1	544.1	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
base poseidon_integration_benchmark	641.0 ± 13.6	633.7	679.2	1.01 ± 0.03
head poseidon_integration_benchmark	637.3 ± 12.6	624.2	658.6	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
base secp_integration_benchmark	1.890 ± 0.029	1.865	1.950	1.01 ± 0.02
head secp_integration_benchmark	1.874 ± 0.016	1.855	1.907	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
base set_integration_benchmark	678.6 ± 2.8	675.8	685.9	1.03 ± 0.01
head set_integration_benchmark	660.1 ± 5.7	654.5	671.3	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
base uint256_integration_benchmark	4.276 ± 0.028	4.218	4.306	1.01 ± 0.01
head uint256_integration_benchmark	4.229 ± 0.035	4.182	4.282	1.00

[DavidLevitGurevich]

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @fmoletta, @gabrielbosio, @igaray, @juanbono, @Oppen, @pefontana, and @YairVaknin-starkware)

---

CHANGELOG.md line 5 at r1 (raw file):

#### Upcoming Changes

- feat: Limited padding of builtin segments to >=16 [#1981](https://github.com/lambdaclass/cairo-vm/pull/1981)

why so many changes in this file?

---

vm/src/vm/runners/builtin_runner/mod.rs line 59 at r1 (raw file):

use super::cairo_pie::BuiltinAdditionalData;

const MIN_N_INSTANCES_IN_BUILTIN_SEGMENT: usize = 0;

when will you change it to 16?

[yuvalsw]

Reviewable status: 1 of 2 files reviewed, 2 unresolved discussions (waiting on @DavidLevitGurevich, @fmoletta, @gabrielbosio, @igaray, @juanbono, @Oppen, @pefontana, and @YairVaknin-starkware)

---

CHANGELOG.md line 5 at r1 (raw file):

Previously, DavidLevitGurevich wrote…

why so many changes in this file?

Done.

[yuvalsw]

vm/src/vm/runners/builtin_runner/mod.rs line 59 at r1 (raw file):

Previously, DavidLevitGurevich wrote…

when will you change it to 16?

Oops... Changed it to 0 to test something, forgot to change back

[YairVaknin-starkware]

Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @DavidLevitGurevich, @fmoletta, @gabrielbosio, @igaray, @juanbono, @Oppen, and @pefontana)

---

vm/src/vm/runners/builtin_runner/mod.rs line 59 at r3 (raw file):

use super::cairo_pie::BuiltinAdditionalData;

const MIN_N_INSTANCES_IN_BUILTIN_SEGMENT: usize = 16;

Doc (or maybe even assert somewhere) that this is a power of 2. Later code assumes it.

Code quote:

const MIN_N_INSTANCES_IN_BUILTIN_SEGMENT: usize = 16;

[yuvalsw]

Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @DavidLevitGurevich, @fmoletta, @gabrielbosio, @igaray, @juanbono, @Oppen, @pefontana, and @YairVaknin-starkware)

---

vm/src/vm/runners/builtin_runner/mod.rs line 59 at r3 (raw file):

Previously, YairVaknin-starkware wrote…

Doc (or maybe even assert somewhere) that this is a power of 2. Later code assumes it.

Done.

[YairVaknin-starkware]

Reviewable status: 0 of 2 files reviewed, 4 unresolved discussions (waiting on @DavidLevitGurevich, @fmoletta, @gabrielbosio, @igaray, @juanbono, @Oppen, and @pefontana)

---

vm/src/vm/runners/builtin_runner/mod.rs line 61 at r4 (raw file):

const MIN_N_INSTANCES_IN_BUILTIN_SEGMENT: usize = 16;

// Assert MIN_N_INSTANCES_IN_BUILTIN_SEGMENT is a power of 2.

Suggestion:

// Assert MIN_N_INSTANCES_IN_BUILTIN_SEGMENT is a power of 2.
const _: () = {
    assert!(MIN_N_INSTANCES_IN_BUILTIN_SEGMENT.is_power_of_two());
};

[YairVaknin-starkware]

vm/src/vm/runners/builtin_runner/mod.rs line 61 at r4 (raw file):

const MIN_N_INSTANCES_IN_BUILTIN_SEGMENT: usize = 16;

// Assert MIN_N_INSTANCES_IN_BUILTIN_SEGMENT is a power of 2.

Also, note the CI failures. They were present before you pushed the above change, but you didn't attend to it, so just making sure you noticed.

[DavidLevitGurevich]

Reviewed 1 of 1 files at r2, 1 of 1 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @fmoletta, @gabrielbosio, @igaray, @juanbono, @Oppen, @pefontana, @YairVaknin-starkware, and @yuvalsw)

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.53%. Comparing base (c71b496) to head (f4ec6aa).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1981   +/-   ##
=======================================
  Coverage   96.53%   96.53%           
=======================================
  Files         103      103           
  Lines       42612    42619    +7     
=======================================
+ Hits        41135    41142    +7     
  Misses       1477     1477           

☔ View full report in Codecov by Sentry.
📢 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.

[yuvalsw]

Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @DavidLevitGurevich, @fmoletta, @gabrielbosio, @igaray, @juanbono, @noaov1, @Oppen, @pefontana, and @YairVaknin-starkware)

---

vm/src/vm/runners/builtin_runner/mod.rs line 61 at r4 (raw file):

Previously, YairVaknin-starkware wrote…

Also, note the CI failures. They were present before you pushed the above change, but you didn't attend to it, so just making sure you noticed.

Done.

[YairVaknin-starkware]

Reviewable status: 0 of 2 files reviewed, all discussions resolved (waiting on @DavidLevitGurevich, @fmoletta, @gabrielbosio, @igaray, @juanbono, @noaov1, @Oppen, and @pefontana)

[gabrielbosio]

Hi, @yuvalsw! Please add context in the description of the PR so that we know why is this change necessary.

[noaov1]

Reviewed 1 of 1 files at r6, 1 of 1 files at r8, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @fmoletta, @gabrielbosio, @igaray, @juanbono, @Oppen, and @pefontana)