Limited padding of segments to >=16 #1981

yuvalsw · 2025-02-27T22:21:01Z

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 · 2025-02-27T22:30:01Z

**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 ± σ):     27.178 s ±  0.031 s    [User: 26.425 s, System: 0.752 s]
  Range (min … max):   27.156 s … 27.200 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 1
  Time (mean ± σ):     26.525 s ±  0.018 s    [User: 25.781 s, System: 0.743 s]
  Range (min … max):   26.513 s … 26.538 s    2 runs
 
Summary
  hyper_threading_pr threads: 1 ran
    1.02 ± 0.00 times faster than hyper_threading_main 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.955 s ±  0.033 s    [User: 26.437 s, System: 0.752 s]
  Range (min … max):   14.932 s … 14.978 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 2
  Time (mean ± σ):     14.847 s ±  0.208 s    [User: 26.100 s, System: 0.758 s]
  Range (min … max):   14.701 s … 14.994 s    2 runs
 
Summary
  hyper_threading_pr threads: 2 ran
    1.01 ± 0.01 times faster than hyper_threading_main 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.990 s ±  0.301 s    [User: 38.838 s, System: 0.922 s]
  Range (min … max):   10.777 s … 11.203 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 4
  Time (mean ± σ):     11.183 s ±  0.028 s    [User: 38.701 s, System: 0.912 s]
  Range (min … max):   11.164 s … 11.203 s    2 runs
 
Summary
  hyper_threading_main threads: 4 ran
    1.02 ± 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 ± σ):     10.733 s ±  0.357 s    [User: 39.695 s, System: 0.966 s]
  Range (min … max):   10.481 s … 10.986 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 6
  Time (mean ± σ):     10.668 s ±  0.110 s    [User: 39.027 s, System: 0.939 s]
  Range (min … max):   10.591 s … 10.746 s    2 runs
 
Summary
  hyper_threading_pr threads: 6 ran
    1.01 ± 0.04 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.835 s ±  0.137 s    [User: 39.689 s, System: 0.931 s]
  Range (min … max):   10.738 s … 10.932 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 8
  Time (mean ± σ):     10.643 s ±  0.142 s    [User: 39.338 s, System: 0.932 s]
  Range (min … max):   10.542 s … 10.743 s    2 runs
 
Summary
  hyper_threading_pr threads: 8 ran
    1.02 ± 0.02 times faster than hyper_threading_main 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.711 s ±  0.008 s    [User: 39.987 s, System: 1.058 s]
  Range (min … max):   10.705 s … 10.716 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 16
  Time (mean ± σ):     10.487 s ±  0.012 s    [User: 39.837 s, System: 1.045 s]
  Range (min … max):   10.479 s … 10.496 s    2 runs
 
Summary
  hyper_threading_pr threads: 16 ran
    1.02 ± 0.00 times faster than hyper_threading_main threads: 16

github-actions · 2025-02-27T22:40:50Z

Benchmark Results for unmodified programs 🚀

Command	Mean [s]	Min [s]	Max [s]	Relative
`base big_factorial`	2.181 ± 0.013	2.166	2.213	1.00
`head big_factorial`	2.189 ± 0.021	2.159	2.231	1.00 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
`base big_fibonacci`	2.114 ± 0.008	2.099	2.123	1.00 ± 0.01
`head big_fibonacci`	2.104 ± 0.012	2.078	2.118	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base blake2s_integration_benchmark`	7.892 ± 0.137	7.764	8.223	1.02 ± 0.02
`head blake2s_integration_benchmark`	7.748 ± 0.125	7.624	7.950	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base compare_arrays_200000`	2.246 ± 0.024	2.217	2.286	1.00
`head compare_arrays_200000`	2.246 ± 0.010	2.234	2.264	1.00 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
`base dict_integration_benchmark`	1.493 ± 0.017	1.474	1.520	1.01 ± 0.01
`head dict_integration_benchmark`	1.482 ± 0.007	1.472	1.496	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base field_arithmetic_get_square_benchmark`	1.259 ± 0.012	1.247	1.283	1.01 ± 0.01
`head field_arithmetic_get_square_benchmark`	1.247 ± 0.009	1.232	1.257	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base integration_builtins`	7.902 ± 0.070	7.814	8.000	1.01 ± 0.01
`head integration_builtins`	7.837 ± 0.067	7.733	7.910	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base keccak_integration_benchmark`	8.122 ± 0.070	8.046	8.253	1.00
`head keccak_integration_benchmark`	8.155 ± 0.125	8.057	8.475	1.00 ± 0.02

Command	Mean [s]	Min [s]	Max [s]	Relative
`base linear_search`	2.221 ± 0.009	2.205	2.232	1.00 ± 0.02
`head linear_search`	2.219 ± 0.039	2.190	2.324	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base math_cmp_and_pow_integration_benchmark`	1.581 ± 0.022	1.556	1.629	1.01 ± 0.02
`head math_cmp_and_pow_integration_benchmark`	1.558 ± 0.014	1.540	1.585	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base math_integration_benchmark`	1.516 ± 0.006	1.509	1.526	1.01 ± 0.01
`head math_integration_benchmark`	1.498 ± 0.013	1.489	1.534	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base memory_integration_benchmark`	1.260 ± 0.021	1.243	1.315	1.01 ± 0.02
`head memory_integration_benchmark`	1.252 ± 0.005	1.243	1.259	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base operations_with_data_structures_benchmarks`	1.631 ± 0.013	1.612	1.659	1.01 ± 0.01
`head operations_with_data_structures_benchmarks`	1.614 ± 0.006	1.605	1.627	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base pedersen`	541.2 ± 1.0	539.4	543.6	1.00 ± 0.00
`head pedersen`	539.8 ± 1.1	538.0	541.3	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base poseidon_integration_benchmark`	651.5 ± 3.5	646.6	658.7	1.00 ± 0.01
`head poseidon_integration_benchmark`	649.2 ± 4.4	641.7	656.2	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base secp_integration_benchmark`	1.912 ± 0.021	1.898	1.969	1.01 ± 0.01
`head secp_integration_benchmark`	1.890 ± 0.017	1.873	1.927	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base set_integration_benchmark`	686.3 ± 7.8	681.1	707.0	1.07 ± 0.01
`head set_integration_benchmark`	640.7 ± 1.4	638.6	643.7	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base uint256_integration_benchmark`	4.384 ± 0.025	4.347	4.432	1.01 ± 0.01
`head uint256_integration_benchmark`	4.328 ± 0.058	4.254	4.452	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 · 2025-03-03T08:17:12Z

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 · 2025-03-05T19:10:50Z

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.

yuvalsw force-pushed the yuval/pad_16 branch from 69a6c82 to e96f89e Compare February 27, 2025 22:22

yuvalsw force-pushed the yuval/pad_16 branch from e96f89e to a3dadbf Compare February 27, 2025 23:43

yuvalsw requested a review from YairVaknin-starkware February 27, 2025 23:44

yuvalsw marked this pull request as ready for review February 27, 2025 23:45

yuvalsw requested review from igaray, Oppen, fmoletta, juanbono, pefontana and gabrielbosio as code owners February 27, 2025 23:45

DavidLevitGurevich reviewed Mar 2, 2025

View reviewed changes

yuvalsw force-pushed the yuval/pad_16 branch from a3dadbf to c3130f2 Compare March 3, 2025 08:16

yuvalsw commented Mar 3, 2025

View reviewed changes

yuvalsw force-pushed the yuval/pad_16 branch from c3130f2 to 187fa4e Compare March 3, 2025 08:16

YairVaknin-starkware requested changes Mar 4, 2025

View reviewed changes

Limited padding of segments to >=16

87d6531

yuvalsw force-pushed the yuval/pad_16 branch from 187fa4e to 87d6531 Compare March 4, 2025 15:06

yuvalsw commented Mar 4, 2025

View reviewed changes

YairVaknin-starkware requested changes Mar 5, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Limited padding of segments to >=16 #1981

Limited padding of segments to >=16 #1981

yuvalsw commented Feb 27, 2025 •

edited

Loading

github-actions bot commented Feb 27, 2025 •

edited

Loading

github-actions bot commented Feb 27, 2025 •

edited

Loading

DavidLevitGurevich left a comment

yuvalsw left a comment

yuvalsw commented Mar 3, 2025

YairVaknin-starkware left a comment

yuvalsw left a comment

YairVaknin-starkware left a comment

YairVaknin-starkware commented Mar 5, 2025

Limited padding of segments to >=16 #1981

Are you sure you want to change the base?

Limited padding of segments to >=16 #1981

Conversation

yuvalsw commented Feb 27, 2025 • edited Loading

TITLE

Description

Checklist

github-actions bot commented Feb 27, 2025 • edited Loading

github-actions bot commented Feb 27, 2025 • edited Loading

DavidLevitGurevich left a comment

Choose a reason for hiding this comment

yuvalsw left a comment

Choose a reason for hiding this comment

yuvalsw commented Mar 3, 2025

YairVaknin-starkware left a comment

Choose a reason for hiding this comment

yuvalsw left a comment

Choose a reason for hiding this comment

YairVaknin-starkware left a comment

Choose a reason for hiding this comment

YairVaknin-starkware commented Mar 5, 2025

yuvalsw commented Feb 27, 2025 •

edited

Loading

github-actions bot commented Feb 27, 2025 •

edited

Loading

github-actions bot commented Feb 27, 2025 •

edited

Loading