Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor syscall handler meta (aka. remove it completely). #479

Merged
merged 4 commits into from
Mar 22, 2024

Conversation

azteca1998
Copy link
Collaborator

The SyscallHandlerMeta was used to store the pointer to the Starknet syscall handler. We don't need it to compile the program therefore it is unnecessary. The syscall handler is now provided to the invoke_* functions directly, which solves some of the issues we have with duplicated MetadataStorages.

Checklist

  • Linked to Github Issue
  • Unit tests added
  • Integration tests added.
  • This change requires new documentation.
    • Documentation has been added/updated.

@azteca1998 azteca1998 marked this pull request as ready for review March 15, 2024 17:43
Copy link

github-actions bot commented Mar 15, 2024

Benchmarking results

Benchmark for program factorial_2M

Open benchmarks
Command Mean [s] Min [s] Max [s] Relative
Cairo-vm (Rust, Cairo 1) 13.538 ± 0.062 13.471 13.637 31.03 ± 0.34
cairo-native (embedded AOT) 1.587 ± 0.018 1.564 1.617 3.64 ± 0.06
cairo-native (embedded JIT using LLVM's ORC Engine) 1.542 ± 0.004 1.534 1.546 3.54 ± 0.04
cairo-native (standalone AOT) 0.636 ± 0.001 0.634 0.638 1.46 ± 0.01
cairo-native (standalone AOT with -march=native) 0.436 ± 0.004 0.434 0.448 1.00

Benchmark for program fib_2M

Open benchmarks
Command Mean [s] Min [s] Max [s] Relative
Cairo-vm (Rust, Cairo 1) 13.005 ± 0.026 12.955 13.046 1603.34 ± 19.81
cairo-native (embedded AOT) 1.095 ± 0.010 1.085 1.116 134.96 ± 2.07
cairo-native (embedded JIT using LLVM's ORC Engine) 1.133 ± 0.023 1.102 1.181 139.68 ± 3.32
cairo-native (standalone AOT) 0.008 ± 0.000 0.008 0.009 1.02 ± 0.02
cairo-native (standalone AOT with -march=native) 0.008 ± 0.000 0.008 0.009 1.00

Benchmark for program logistic_map

Open benchmarks
Command Mean [s] Min [s] Max [s] Relative
Cairo-vm (Rust, Cairo 1) 1.870 ± 0.015 1.853 1.895 27.56 ± 0.24
cairo-native (embedded AOT) 1.228 ± 0.016 1.206 1.257 18.09 ± 0.25
cairo-native (embedded JIT using LLVM's ORC Engine) 1.456 ± 0.013 1.436 1.482 21.46 ± 0.21
cairo-native (standalone AOT) 0.112 ± 0.000 0.111 0.112 1.65 ± 0.01
cairo-native (standalone AOT with -march=native) 0.068 ± 0.000 0.068 0.069 1.00

@codecov-commenter
Copy link

codecov-commenter commented Mar 15, 2024

Codecov Report

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

Project coverage is 79.51%. Comparing base (8fddcd3) to head (7a7fe47).

Files Patch % Lines
src/starknet.rs 0.00% 139 Missing ⚠️
src/executor.rs 32.35% 23 Missing ⚠️
src/executor/aot.rs 0.00% 23 Missing ⚠️
src/libfuncs/starknet.rs 8.33% 11 Missing ⚠️
src/bin/cairo-native-dump.rs 0.00% 8 Missing ⚠️
src/context.rs 0.00% 3 Missing ⚠️
src/bin/cairo-native-run.rs 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #479      +/-   ##
==========================================
- Coverage   79.90%   79.51%   -0.39%     
==========================================
  Files         111      110       -1     
  Lines       34248    34399     +151     
==========================================
- Hits        27365    27353      -12     
- Misses       6883     7046     +163     

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

edg-l
edg-l previously approved these changes Mar 18, 2024
@juanbono juanbono added this pull request to the merge queue Mar 22, 2024
Merged via the queue into main with commit 8f7602c Mar 22, 2024
9 checks passed
@juanbono juanbono deleted the refactor-syscall-handler-meta branch March 22, 2024 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants