Use `BuiltinName` enum instead of string representation #1722

fmoletta · 2024-04-17T20:48:37Z

Replaces all uses of String & &'static str representing builtin names with BuiltinName enum.
Implements methods to convert BuiltinName to and from str with and without suffix, adding documentation & examples to reduce confusion for users due to the breaking changes (To see all breaking changes, checkout the CHANGELOG entry for this PR).
Implements generic serialization & deserialization for HashMaps containing BuiltinName that use a suffixed version of the builtin names.

This PR will make enforce valid builtin names across the codebase, removing the need for extra validations, and will be particularly useful for ensuring that cairo pies are valid before running them (#1720)

codecov · 2024-04-17T21:34:32Z

Codecov Report

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

Project coverage is 94.80%. Comparing base (15bf794) to head (b7f982f).

Files	Patch %	Lines
vm/src/types/builtin_name.rs	50.56%	44 Missing ⚠️
vm/src/vm/runners/builtin_runner/range_check.rs	40.00%	3 Missing ⚠️
vm/src/vm/runners/cairo_runner.rs	98.05%	2 Missing ⚠️
vm/src/vm/vm_core.rs	77.77%	2 Missing ⚠️
vm/src/vm/runners/builtin_runner/mod.rs	94.73%	1 Missing ⚠️
vm/src/vm/runners/builtin_runner/modulo.rs	0.00%	1 Missing ⚠️
vm/src/vm/runners/builtin_runner/output.rs	83.33%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1722      +/-   ##
==========================================
- Coverage   94.87%   94.80%   -0.08%     
==========================================
  Files          99      100       +1     
  Lines       38174    38212      +38     
==========================================
+ Hits        36218    36225       +7     
- Misses       1956     1987      +31

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

github-actions · 2024-04-17T21:36:28Z

Benchmark Results for unmodified programs 🚀

Command	Mean [s]	Min [s]	Max [s]	Relative
`base big_factorial`	1.944 ± 0.015	1.928	1.967	1.00
`head big_factorial`	1.969 ± 0.036	1.912	2.024	1.01 ± 0.02

Command	Mean [s]	Min [s]	Max [s]	Relative
`base big_fibonacci`	1.864 ± 0.037	1.847	1.969	1.00
`head big_fibonacci`	1.909 ± 0.088	1.831	2.145	1.02 ± 0.05

Command	Mean [s]	Min [s]	Max [s]	Relative
`base blake2s_integration_benchmark`	6.551 ± 0.079	6.471	6.659	1.00 ± 0.02
`head blake2s_integration_benchmark`	6.549 ± 0.106	6.383	6.703	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base compare_arrays_200000`	1.955 ± 0.013	1.935	1.975	1.01 ± 0.02
`head compare_arrays_200000`	1.944 ± 0.027	1.902	1.972	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base dict_integration_benchmark`	1.310 ± 0.015	1.296	1.344	1.00
`head dict_integration_benchmark`	1.343 ± 0.019	1.311	1.383	1.02 ± 0.02

Command	Mean [s]	Min [s]	Max [s]	Relative
`base field_arithmetic_get_square_benchmark`	1.182 ± 0.015	1.169	1.214	1.00 ± 0.02
`head field_arithmetic_get_square_benchmark`	1.179 ± 0.023	1.151	1.225	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base integration_builtins`	6.816 ± 0.379	6.585	7.869	1.03 ± 0.06
`head integration_builtins`	6.647 ± 0.108	6.539	6.831	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base keccak_integration_benchmark`	6.980 ± 0.359	6.732	7.952	1.03 ± 0.05
`head keccak_integration_benchmark`	6.796 ± 0.099	6.689	6.952	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base linear_search`	2.031 ± 0.073	1.970	2.213	1.00
`head linear_search`	2.034 ± 0.102	1.965	2.311	1.00 ± 0.06

Command	Mean [s]	Min [s]	Max [s]	Relative
`base math_cmp_and_pow_integration_benchmark`	1.753 ± 0.017	1.738	1.796	1.02 ± 0.02
`head math_cmp_and_pow_integration_benchmark`	1.727 ± 0.024	1.696	1.768	1.00

Command	Mean [s]	Min [s]	Max [s]	Relative
`base math_integration_benchmark`	1.493 ± 0.007	1.477	1.504	1.00
`head math_integration_benchmark`	1.493 ± 0.018	1.472	1.519	1.00 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
`base memory_integration_benchmark`	1.169 ± 0.028	1.147	1.245	1.00
`head memory_integration_benchmark`	1.186 ± 0.032	1.157	1.246	1.02 ± 0.04

Command	Mean [s]	Min [s]	Max [s]	Relative
`base operations_with_data_structures_benchmarks`	1.784 ± 0.007	1.775	1.796	1.00
`head operations_with_data_structures_benchmarks`	1.805 ± 0.021	1.785	1.859	1.01 ± 0.01

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base pedersen`	512.6 ± 4.8	506.2	521.0	1.01 ± 0.01
`head pedersen`	508.4 ± 4.4	501.2	514.8	1.00

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base poseidon_integration_benchmark`	956.4 ± 8.9	950.1	980.6	1.00
`head poseidon_integration_benchmark`	968.3 ± 7.7	959.5	981.3	1.01 ± 0.01

Command	Mean [s]	Min [s]	Max [s]	Relative
`base secp_integration_benchmark`	1.750 ± 0.013	1.734	1.781	1.00
`head secp_integration_benchmark`	1.794 ± 0.038	1.751	1.890	1.03 ± 0.02

Command	Mean [ms]	Min [ms]	Max [ms]	Relative
`base set_integration_benchmark`	694.5 ± 4.2	689.7	703.7	1.00
`head set_integration_benchmark`	727.2 ± 19.2	713.7	778.3	1.05 ± 0.03

Command	Mean [s]	Min [s]	Max [s]	Relative
`base uint256_integration_benchmark`	3.856 ± 0.027	3.818	3.902	1.01 ± 0.02
`head uint256_integration_benchmark`	3.806 ± 0.051	3.764	3.905	1.00

…ro-vm into use-builtin-name

github-actions · 2024-04-18T15:51:55Z

**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.640 s ±  0.031 s    [User: 22.872 s, System: 0.767 s]
  Range (min … max):   23.618 s … 23.662 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 1
  Time (mean ± σ):     23.728 s ±  0.092 s    [User: 23.025 s, System: 0.702 s]
  Range (min … max):   23.663 s … 23.794 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 ± σ):     12.302 s ±  0.028 s    [User: 23.129 s, System: 0.742 s]
  Range (min … max):   12.282 s … 12.321 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 2
  Time (mean ± σ):     12.315 s ±  0.021 s    [User: 23.263 s, System: 0.654 s]
  Range (min … max):   12.300 s … 12.330 s    2 runs
 
Summary
  'hyper_threading_main threads: 2' ran
    1.00 ± 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.714 s ±  0.067 s    [User: 34.958 s, System: 0.929 s]
  Range (min … max):    9.666 s …  9.761 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 4
  Time (mean ± σ):      9.905 s ±  0.606 s    [User: 35.242 s, System: 0.947 s]
  Range (min … max):    9.477 s … 10.334 s    2 runs
 
Summary
  'hyper_threading_main threads: 4' ran
    1.02 ± 0.06 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.611 s ±  0.162 s    [User: 34.869 s, System: 1.030 s]
  Range (min … max):    9.496 s …  9.725 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 6
  Time (mean ± σ):      9.740 s ±  0.014 s    [User: 35.263 s, System: 0.982 s]
  Range (min … max):    9.730 s …  9.750 s    2 runs
 
Summary
  'hyper_threading_main threads: 6' ran
    1.01 ± 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.601 s ±  0.309 s    [User: 35.356 s, System: 1.097 s]
  Range (min … max):    9.383 s …  9.820 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 8
  Time (mean ± σ):      9.507 s ±  0.117 s    [User: 35.730 s, System: 1.050 s]
  Range (min … max):    9.425 s …  9.589 s    2 runs
 
Summary
  'hyper_threading_pr threads: 8' ran
    1.01 ± 0.03 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.476 s ±  0.081 s    [User: 35.563 s, System: 1.084 s]
  Range (min … max):    9.419 s …  9.533 s    2 runs
 
Benchmark 2: hyper_threading_pr threads: 16
  Time (mean ± σ):      9.693 s ±  0.220 s    [User: 35.870 s, System: 1.084 s]
  Range (min … max):    9.537 s …  9.849 s    2 runs
 
Summary
  'hyper_threading_main threads: 16' ran
    1.02 ± 0.02 times faster than 'hyper_threading_pr threads: 16'

…se-builtin-name

cairo1-run/src/cairo_run.rs

Co-authored-by: Juan Bono <[email protected]>

fmoletta added 5 commits April 17, 2024 15:10

Save progres

3f99b46

Progress

b071346

Progress

9ce9aea

Clippy

c79c02a

Impl generic serialization for suffixed ver

f8e8029

fmoletta added 7 commits April 18, 2024 11:06

Implement custom generic deserialization

e821c04

Rename module

3c4a417

Complete API

cac3156

Add docs & examples

72c1a6b

Fix docs

2652145

Remove reference from method

4dd4e2b

Add clippy allow

fc31dc6

fmoletta changed the title ~~[WIP] Use builtin_name enum instead of string representation~~ [WIP] Use BuiltinName enum instead of string representation Apr 18, 2024

fmoletta and others added 3 commits April 18, 2024 12:25

Add changelog entry

898bb0e

Add no-std import

7799173

Merge branch 'main' into use-builtin-name

922f963

fmoletta changed the title ~~[WIP] Use BuiltinName enum instead of string representation~~ Use BuiltinName enum instead of string representation Apr 18, 2024

fmoletta added 2 commits April 18, 2024 12:46

Fix

c49bb4b

Merge branch 'use-builtin-name' of https://github.com/lambdaclass/cai…

55d70b7

…ro-vm into use-builtin-name

fmoletta added 4 commits April 18, 2024 13:05

fmt

107a637

Use a wrapper for CairoPie additional data

f8d0082

Update changelog

92e69ee

Fix tests + fmt

5b54cfa

fmoletta marked this pull request as ready for review April 18, 2024 19:09

fmoletta requested review from igaray, Oppen and juanbono as code owners April 18, 2024 19:09

fmoletta requested a review from pefontana as a code owner April 18, 2024 19:09

Oppen approved these changes Apr 18, 2024

View reviewed changes

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

4730d7c

…se-builtin-name

juanbono reviewed Apr 19, 2024

View reviewed changes

cairo1-run/src/cairo_run.rs Outdated Show resolved Hide resolved

Update cairo1-run/src/cairo_run.rs

cd6657f

Co-authored-by: Juan Bono <[email protected]>

juanbono approved these changes Apr 19, 2024

View reviewed changes

juanbono enabled auto-merge April 19, 2024 15:10

Merge branch 'main' into use-builtin-name

b7f982f

juanbono added this pull request to the merge queue Apr 19, 2024

Merged via the queue into main with commit 7796f24 Apr 19, 2024
70 of 71 checks passed

juanbono deleted the use-builtin-name branch April 19, 2024 17:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use `BuiltinName` enum instead of string representation #1722

Use `BuiltinName` enum instead of string representation #1722

fmoletta commented Apr 17, 2024 •

edited

Loading

codecov bot commented Apr 17, 2024 •

edited

Loading

github-actions bot commented Apr 17, 2024 •

edited

Loading

github-actions bot commented Apr 18, 2024 •

edited

Loading

Use BuiltinName enum instead of string representation #1722

Use BuiltinName enum instead of string representation #1722

Conversation

fmoletta commented Apr 17, 2024 • edited Loading

codecov bot commented Apr 17, 2024 • edited Loading

Codecov Report

github-actions bot commented Apr 17, 2024 • edited Loading

github-actions bot commented Apr 18, 2024 • edited Loading

Use `BuiltinName` enum instead of string representation #1722

Use `BuiltinName` enum instead of string representation #1722

fmoletta commented Apr 17, 2024 •

edited

Loading

codecov bot commented Apr 17, 2024 •

edited

Loading

github-actions bot commented Apr 17, 2024 •

edited

Loading

github-actions bot commented Apr 18, 2024 •

edited

Loading