Minor `ret` decoding refactor #1925

YairVaknin-starkware · 2025-01-26T12:41:04Z

TITLE

Minor ret decoding refactor

Description

Limit ret opcode decodeing to Cairo0's standards.

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.

github-actions · 2025-01-26T12:50:43Z

**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 ± σ):     32.100 s ±  0.125 s    [User: 31.318 s, System: 0.779 s]
  Range (min … max):   32.012 s … 32.188 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 1
  Time (mean ± σ):     32.588 s ±  0.049 s    [User: 31.809 s, System: 0.776 s]
  Range (min … max):   32.554 s … 32.622 s    2 runs
 
Summary
  hyper_threading_main threads: 1 ran
    1.02 ± 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 ± σ):     17.889 s ±  0.099 s    [User: 31.238 s, System: 0.802 s]
  Range (min … max):   17.819 s … 17.959 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 2
  Time (mean ± σ):     18.042 s ±  0.023 s    [User: 31.547 s, System: 0.805 s]
  Range (min … max):   18.026 s … 18.059 s    2 runs
 
Summary
  hyper_threading_main threads: 2 ran
    1.01 ± 0.01 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 ± σ):     12.409 s ±  0.402 s    [User: 43.695 s, System: 0.920 s]
  Range (min … max):   12.125 s … 12.694 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 4
  Time (mean ± σ):     12.174 s ±  0.439 s    [User: 43.836 s, System: 0.920 s]
  Range (min … max):   11.864 s … 12.484 s    2 runs
 
Summary
  hyper_threading_pr threads: 4 ran
    1.02 ± 0.05 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 ± σ):     12.225 s ±  0.137 s    [User: 44.111 s, System: 0.938 s]
  Range (min … max):   12.128 s … 12.322 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 6
  Time (mean ± σ):     12.200 s ±  0.351 s    [User: 43.848 s, System: 0.918 s]
  Range (min … max):   11.952 s … 12.449 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 ± σ):     11.947 s ±  0.149 s    [User: 44.345 s, System: 0.972 s]
  Range (min … max):   11.842 s … 12.053 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 8
  Time (mean ± σ):     11.907 s ±  0.005 s    [User: 44.155 s, System: 0.960 s]
  Range (min … max):   11.904 s … 11.910 s    2 runs
 
Summary
  hyper_threading_pr threads: 8 ran
    1.00 ± 0.01 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 ± σ):     11.876 s ±  0.286 s    [User: 44.440 s, System: 0.993 s]
  Range (min … max):   11.674 s … 12.079 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 16
  Time (mean ± σ):     12.028 s ±  0.424 s    [User: 44.412 s, System: 1.026 s]
  Range (min … max):   11.728 s … 12.328 s    2 runs
 
Summary
  hyper_threading_main threads: 16 ran
    1.01 ± 0.04 times faster than hyper_threading_pr threads: 16

github-actions · 2025-01-26T13:05:06Z

Benchmark Results for unmodified programs 🚀

Command	Mean [s]	Min [s]	Max [s]	Relative
`base big_factorial`	2.546 ± 0.049	2.496	2.667	1.00
`head big_factorial`	2.608 ± 0.179	2.517	3.104	1.02 ± 0.07

Command	Mean [s]	Min [s]	Max [s]	Relative
`base big_fibonacci`	2.549 ± 0.087	2.432	2.722	1.03 ± 0.04
`head big_fibonacci`	2.478 ± 0.039	2.440	2.547	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base blake2s_integration_benchmark`	9.281 ± 0.144	9.127	9.516	1.00 ± 0.02
`head blake2s_integration_benchmark`	9.272 ± 0.162	9.115	9.684	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base compare_arrays_200000`	2.622 ± 0.044	2.569	2.700	1.00
`head compare_arrays_200000`	2.634 ± 0.018	2.603	2.659	1.00 ± 0.02

Command	Mean [s]	Min [s]	Max [s]	Relative
`base dict_integration_benchmark`	1.743 ± 0.016	1.718	1.758	1.01 ± 0.02
`head dict_integration_benchmark`	1.728 ± 0.021	1.701	1.762	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base field_arithmetic_get_square_benchmark`	1.454 ± 0.028	1.424	1.516	1.00 ± 0.02
`head field_arithmetic_get_square_benchmark`	1.447 ± 0.017	1.434	1.493	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base integration_builtins`	9.410 ± 0.220	9.215	9.986	1.00 ± 0.04
`head integration_builtins`	9.381 ± 0.276	9.116	9.988	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base keccak_integration_benchmark`	9.671 ± 0.095	9.534	9.779	1.00 ± 0.03
`head keccak_integration_benchmark`	9.664 ± 0.296	9.445	10.438	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base linear_search`	2.635 ± 0.070	2.579	2.823	1.01 ± 0.03
`head linear_search`	2.607 ± 0.028	2.563	2.644	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base math_cmp_and_pow_integration_benchmark`	1.778 ± 0.043	1.731	1.866	1.01 ± 0.03
`head math_cmp_and_pow_integration_benchmark`	1.768 ± 0.015	1.745	1.794	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base math_integration_benchmark`	1.725 ± 0.026	1.701	1.788	1.00 ± 0.02
`head math_integration_benchmark`	1.717 ± 0.015	1.694	1.736	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base memory_integration_benchmark`	1.469 ± 0.050	1.434	1.597	1.00
`head memory_integration_benchmark`	1.471 ± 0.031	1.443	1.541	1.00 ± 0.04

Command	Mean [s]	Min [s]	Max [s]	Relative
`base operations_with_data_structures_benchmarks`	1.842 ± 0.027	1.812	1.904	1.00
`head operations_with_data_structures_benchmarks`	1.854 ± 0.027	1.829	1.905	1.01 ± 0.02

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base pedersen`	590.5 ± 4.3	585.5	598.6	1.00
`head pedersen`	595.2 ± 7.6	589.1	615.5	1.01 ± 0.01

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base poseidon_integration_benchmark`	714.2 ± 8.8	704.1	735.3	1.00
`head poseidon_integration_benchmark`	715.2 ± 6.4	708.7	727.0	1.00 ± 0.02

Command	Mean [s]	Min [s]	Max [s]	Relative
`base secp_integration_benchmark`	2.098 ± 0.032	2.066	2.152	1.00 ± 0.02
`head secp_integration_benchmark`	2.092 ± 0.010	2.083	2.116	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base set_integration_benchmark`	693.7 ± 16.1	681.0	733.5	1.00
`head set_integration_benchmark`	726.2 ± 8.9	710.5	740.2	1.05 ± 0.03

Command	Mean [s]	Min [s]	Max [s]	Relative
`base uint256_integration_benchmark`	5.105 ± 0.063	5.029	5.244	1.00
`head uint256_integration_benchmark`	5.175 ± 0.161	5.048	5.484	1.01 ± 0.03

codecov · 2025-01-27T10:22:39Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.35%. Comparing base (fb2b1fe) to head (4825dab).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1925      +/-   ##
==========================================
+ Coverage   96.34%   96.35%   +0.01%     
==========================================
  Files         102      102              
  Lines       40830    41005     +175     
==========================================
+ Hits        39336    39511     +175     
  Misses       1494     1494

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

JulianGCalderon

Hi @YairVaknin-starkware! I left you some small suggestions regarding the test names

vm/src/vm/decoding/decoder.rs

* Minor ret decoding refactor - #1925 cherry pick * lock cargo-llvm-cov * Update test_install runner to macos-13 --------- Co-authored-by: Julián González Calderón <gonzalezcalderonjulian@gmail.com>

YairVaknin-starkware requested review from igaray, Oppen, fmoletta, juanbono, pefontana and gabrielbosio as code owners January 26, 2025 12:41

YairVaknin-starkware force-pushed the yairv/minor_ret_decoding_refactor branch from f8be7c5 to 3b54fa8 Compare January 26, 2025 12:43

YairVaknin-starkware requested a review from OmriEshhar1 January 26, 2025 12:44

YairVaknin-starkware force-pushed the yairv/minor_ret_decoding_refactor branch 2 times, most recently from 8d9e231 to bef74b6 Compare January 27, 2025 09:51

YairVaknin-starkware force-pushed the yairv/minor_ret_decoding_refactor branch 2 times, most recently from 87e39e8 to 5977c76 Compare January 27, 2025 18:01

Minor_ret_decoding_refactor

0f4e53c

YairVaknin-starkware force-pushed the yairv/minor_ret_decoding_refactor branch from 5977c76 to 0f4e53c Compare January 27, 2025 18:44

Merge branch 'main' into yairv/minor_ret_decoding_refactor

8666ca4

igaray approved these changes Jan 27, 2025

View reviewed changes

JulianGCalderon reviewed Jan 27, 2025

View reviewed changes

vm/src/vm/decoding/decoder.rs Outdated Show resolved Hide resolved

vm/src/vm/decoding/decoder.rs Outdated Show resolved Hide resolved

vm/src/vm/decoding/decoder.rs Outdated Show resolved Hide resolved

tests naming fix

4825dab

JulianGCalderon approved these changes Jan 27, 2025

View reviewed changes

YairVaknin-starkware added this pull request to the merge queue Jan 27, 2025

Merged via the queue into main with commit a0f8806 Jan 27, 2025
91 checks passed

YairVaknin-starkware deleted the yairv/minor_ret_decoding_refactor branch January 27, 2025 21:46

YairVaknin-starkware added a commit that referenced this pull request Jan 27, 2025

Minor ret decoding refactor - #1925 cherry pick

faa0876

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Minor `ret` decoding refactor #1925

Minor `ret` decoding refactor #1925

YairVaknin-starkware commented Jan 26, 2025

github-actions bot commented Jan 26, 2025 •

edited

Loading

github-actions bot commented Jan 26, 2025 •

edited

Loading

codecov bot commented Jan 27, 2025 •

edited

Loading

JulianGCalderon left a comment

Minor ret decoding refactor #1925

Minor ret decoding refactor #1925

Conversation

YairVaknin-starkware commented Jan 26, 2025

TITLE

Description

Checklist

github-actions bot commented Jan 26, 2025 • edited Loading

github-actions bot commented Jan 26, 2025 • edited Loading

codecov bot commented Jan 27, 2025 • edited Loading

Codecov Report

JulianGCalderon left a comment

Choose a reason for hiding this comment

Minor `ret` decoding refactor #1925

Minor `ret` decoding refactor #1925

github-actions bot commented Jan 26, 2025 •

edited

Loading

github-actions bot commented Jan 26, 2025 •

edited

Loading

codecov bot commented Jan 27, 2025 •

edited

Loading