Use an enum to represent layout name #1715

fmoletta · 2024-04-16T13:42:02Z

Uses an enum instead of a String to represent cairo layouts.
This makes layout matching faster and improves parsing of cli arguments
String representation is still left in PublicInput struct, this can be changed once #1716 is merged

…ayout-enum

github-actions · 2024-04-16T14:39:05Z

Benchmark Results for unmodified programs 🚀

Command	Mean [s]	Min [s]	Max [s]	Relative
`base big_factorial`	1.913 ± 0.021	1.888	1.965	1.00
`head big_factorial`	1.927 ± 0.058	1.892	2.089	1.01 ± 0.03

Command	Mean [s]	Min [s]	Max [s]	Relative
`base big_fibonacci`	1.849 ± 0.018	1.829	1.882	1.00
`head big_fibonacci`	1.859 ± 0.020	1.840	1.909	1.01 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
`base blake2s_integration_benchmark`	6.564 ± 0.053	6.454	6.626	1.00
`head blake2s_integration_benchmark`	6.618 ± 0.100	6.470	6.808	1.01 ± 0.02

Command	Mean [s]	Min [s]	Max [s]	Relative
`base compare_arrays_200000`	1.921 ± 0.017	1.907	1.962	1.00 ± 0.01
`head compare_arrays_200000`	1.915 ± 0.022	1.897	1.964	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base dict_integration_benchmark`	1.314 ± 0.017	1.298	1.356	1.00 ± 0.02
`head dict_integration_benchmark`	1.308 ± 0.021	1.287	1.347	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base field_arithmetic_get_square_benchmark`	1.175 ± 0.039	1.154	1.284	1.01 ± 0.04
`head field_arithmetic_get_square_benchmark`	1.168 ± 0.017	1.155	1.214	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base integration_builtins`	6.549 ± 0.183	6.433	7.044	1.00 ± 0.03
`head integration_builtins`	6.527 ± 0.076	6.442	6.633	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base keccak_integration_benchmark`	6.785 ± 0.128	6.662	7.056	1.01 ± 0.02
`head keccak_integration_benchmark`	6.700 ± 0.072	6.621	6.804	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base linear_search`	1.929 ± 0.014	1.912	1.965	1.00
`head linear_search`	1.943 ± 0.059	1.908	2.088	1.01 ± 0.03

Command	Mean [s]	Min [s]	Max [s]	Relative
`base math_cmp_and_pow_integration_benchmark`	1.673 ± 0.010	1.661	1.699	1.00
`head math_cmp_and_pow_integration_benchmark`	1.674 ± 0.008	1.660	1.687	1.00 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
`base math_integration_benchmark`	1.472 ± 0.037	1.444	1.543	1.01 ± 0.03
`head math_integration_benchmark`	1.464 ± 0.009	1.454	1.486	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base memory_integration_benchmark`	1.140 ± 0.015	1.127	1.181	1.00
`head memory_integration_benchmark`	1.144 ± 0.013	1.133	1.180	1.00 ± 0.02

Command	Mean [s]	Min [s]	Max [s]	Relative
`base operations_with_data_structures_benchmarks`	1.782 ± 0.032	1.760	1.857	1.01 ± 0.02
`head operations_with_data_structures_benchmarks`	1.766 ± 0.026	1.748	1.832	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base pedersen`	504.7 ± 2.4	501.8	510.3	1.00
`head pedersen`	513.8 ± 12.0	503.8	533.1	1.02 ± 0.02

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base poseidon_integration_benchmark`	954.8 ± 33.8	939.3	1050.2	1.00 ± 0.04
`head poseidon_integration_benchmark`	954.8 ± 10.5	943.6	979.9	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base secp_integration_benchmark`	1.718 ± 0.006	1.707	1.724	1.00
`head secp_integration_benchmark`	1.722 ± 0.011	1.707	1.737	1.00 ± 0.01

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base set_integration_benchmark`	692.8 ± 13.5	684.0	728.7	1.01 ± 0.02
`head set_integration_benchmark`	685.4 ± 1.2	683.7	687.1	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base uint256_integration_benchmark`	3.812 ± 0.131	3.741	4.179	1.01 ± 0.04
`head uint256_integration_benchmark`	3.787 ± 0.049	3.735	3.895	1.00

codecov · 2024-04-16T14:39:53Z

Codecov Report

Attention: Patch coverage is 92.56198% with 9 lines in your changes are missing coverage. Please review.

Project coverage is 94.93%. Comparing base (4b17118) to head (a6ed611).

Files	Patch %	Lines
vm/src/types/layout_name.rs	88.57%	4 Missing ⚠️
vm/src/vm/runners/cairo_runner.rs	94.54%	3 Missing ⚠️
cairo-vm-cli/src/main.rs	50.00%	1 Missing ⚠️
cairo1-run/src/main.rs	50.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1715      +/-   ##
==========================================
+ Coverage   94.91%   94.93%   +0.02%     
==========================================
  Files          98       99       +1     
  Lines       38170    38147      -23     
==========================================
- Hits        36228    36216      -12     
+ Misses       1942     1931      -11

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

vm/src/types/layout_name.rs

Co-authored-by: Mario Rugiero <mario.rugiero@lambdaclass.com>

juanbono

Great!

github-actions · 2024-04-17T18:51:46Z

**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 ± σ):     23.581 s ±  0.043 s    [User: 22.872 s, System: 0.707 s]
  Range (min … max):   23.550 s … 23.611 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 1
  Time (mean ± σ):     23.513 s ±  0.004 s    [User: 22.767 s, System: 0.744 s]
  Range (min … max):   23.510 s … 23.515 s    2 runs
 
Summary
  'hyper_threading_pr threads: 1' ran
    1.00 ± 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 ± σ):     12.281 s ±  0.034 s    [User: 23.009 s, System: 0.766 s]
  Range (min … max):   12.257 s … 12.305 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 2
  Time (mean ± σ):     12.253 s ±  0.010 s    [User: 23.091 s, System: 0.696 s]
  Range (min … max):   12.246 s … 12.260 s    2 runs
 
Summary
  'hyper_threading_pr threads: 2' ran
    1.00 ± 0.00 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 ± σ):      9.652 s ±  0.239 s    [User: 34.886 s, System: 0.899 s]
  Range (min … max):    9.483 s …  9.821 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 4
  Time (mean ± σ):      9.759 s ±  0.021 s    [User: 34.549 s, System: 0.887 s]
  Range (min … max):    9.744 s …  9.775 s    2 runs
 
Summary
  'hyper_threading_main threads: 4' ran
    1.01 ± 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.581 s ±  0.172 s    [User: 34.793 s, System: 0.975 s]
  Range (min … max):    9.460 s …  9.703 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 6
  Time (mean ± σ):      9.597 s ±  0.070 s    [User: 34.612 s, System: 1.003 s]
  Range (min … max):    9.548 s …  9.647 s    2 runs
 
Summary
  'hyper_threading_main threads: 6' ran
    1.00 ± 0.02 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.604 s ±  0.079 s    [User: 34.899 s, System: 0.957 s]
  Range (min … max):    9.549 s …  9.660 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 8
  Time (mean ± σ):      9.485 s ±  0.052 s    [User: 34.897 s, System: 0.942 s]
  Range (min … max):    9.448 s …  9.522 s    2 runs
 
Summary
  'hyper_threading_pr threads: 8' ran
    1.01 ± 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 ± σ):      9.618 s ±  0.194 s    [User: 34.966 s, System: 1.039 s]
  Range (min … max):    9.481 s …  9.755 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 16
  Time (mean ± σ):      9.616 s ±  0.084 s    [User: 34.908 s, System: 1.040 s]
  Range (min … max):    9.557 s …  9.675 s    2 runs
 
Summary
  'hyper_threading_pr threads: 16' ran
    1.00 ± 0.02 times faster than 'hyper_threading_main threads: 16'

pefontana

LGTM!

fmoletta added 13 commits April 15, 2024 10:28

Use enum for LayoutName

248af03

Move LayoutName to its own module + fix tests

9f16091

Fix clap enum parsing

a1b7ab3

Fix tests

29ed4d5

Fix test & doc

2d555ca

Manually implement ValueEnum

6455b6b

fmt

b294733

Simplify code

c068b79

Move arbitrary impl to module

961e661

Merge branch 'main' of https://github.com/lambdaclass/cairo-vm into l…

185d48b

…ayout-enum

Fix benchmark code

785cab6

Add changelog entry

1fa267c

Remove unnecessary impl

69bdf18

fmoletta changed the title ~~[WIP] Use an enum to represent layout name~~ Use an enum to represent layout name Apr 16, 2024

fmoletta marked this pull request as ready for review April 16, 2024 14:44

fmoletta requested review from igaray, Oppen, juanbono and pefontana as code owners April 16, 2024 14:44

fmoletta mentioned this pull request Apr 16, 2024

Set dynamic params as null by default on air public input #1716

Merged

Oppen reviewed Apr 16, 2024

View reviewed changes

vm/src/types/layout_name.rs Outdated Show resolved Hide resolved

Use value instead of reference

5a2a513

Co-authored-by: Mario Rugiero <mario.rugiero@lambdaclass.com>

juanbono approved these changes Apr 17, 2024

View reviewed changes

Merge branch 'main' into layout-enum

9464513

fmoletta and others added 2 commits April 18, 2024 11:51

Merge branch 'main' into layout-enum

5fb45da

Fix test added by merge

a6ed611

fmoletta mentioned this pull request Apr 18, 2024

Add recursive_with_poseidon layout #1724

Merged

pefontana approved these changes Apr 18, 2024

View reviewed changes

pefontana added this pull request to the merge queue Apr 18, 2024

Merged via the queue into main with commit 932986c Apr 18, 2024
70 of 71 checks passed

pefontana deleted the layout-enum branch April 18, 2024 20:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use an enum to represent layout name #1715

Use an enum to represent layout name #1715

fmoletta commented Apr 16, 2024 •

edited

github-actions bot commented Apr 16, 2024 •

edited

codecov bot commented Apr 16, 2024 •

edited

juanbono left a comment

github-actions bot commented Apr 17, 2024 •

edited

pefontana left a comment

Use an enum to represent layout name #1715

Use an enum to represent layout name #1715

Conversation

fmoletta commented Apr 16, 2024 • edited

github-actions bot commented Apr 16, 2024 • edited

codecov bot commented Apr 16, 2024 • edited

Codecov Report

juanbono left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 17, 2024 • edited

pefontana left a comment

Choose a reason for hiding this comment

fmoletta commented Apr 16, 2024 •

edited

github-actions bot commented Apr 16, 2024 •

edited

codecov bot commented Apr 16, 2024 •

edited

github-actions bot commented Apr 17, 2024 •

edited