-
Notifications
You must be signed in to change notification settings - Fork 49
moved array metadata to be behind a single pointer refrence #1551
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
base: main
Are you sure you want to change the base?
Conversation
478213a to
363e838
Compare
Benchmark results Main vs HEAD.Base
Head
Base
Head
Base
Head
Base
Head
Base
Head
Base
Head
|
Benchmarking resultsBenchmark for program
|
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
11.142 ± 0.084 | 11.036 | 11.251 | 5.64 ± 0.07 |
cairo-native (embedded AOT) |
1.975 ± 0.018 | 1.949 | 2.008 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2.067 ± 0.016 | 2.047 | 2.096 | 1.05 ± 0.01 |
Benchmark for program dict_snapshot
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
570.1 ± 4.6 | 562.4 | 575.8 | 1.00 |
cairo-native (embedded AOT) |
1665.0 ± 17.9 | 1636.7 | 1686.8 | 2.92 ± 0.04 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1785.1 ± 16.1 | 1763.5 | 1811.3 | 3.13 ± 0.04 |
Benchmark for program factorial_2M
Open benchmarks
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
4.962 ± 0.021 | 4.939 | 5.015 | 2.34 ± 0.05 |
cairo-native (embedded AOT) |
2.117 ± 0.042 | 2.081 | 2.231 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2.179 ± 0.016 | 2.155 | 2.202 | 1.03 ± 0.02 |
Benchmark for program fib_2M
Open benchmarks
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
4.855 ± 0.017 | 4.836 | 4.898 | 2.92 ± 0.03 |
cairo-native (embedded AOT) |
1.661 ± 0.018 | 1.636 | 1.689 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1.720 ± 0.013 | 1.702 | 1.743 | 1.04 ± 0.01 |
Benchmark for program linear_search
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
619.3 ± 5.6 | 611.4 | 627.6 | 1.00 |
cairo-native (embedded AOT) |
1711.8 ± 23.7 | 1682.7 | 1749.4 | 2.76 ± 0.05 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1837.1 ± 9.9 | 1822.2 | 1853.4 | 2.97 ± 0.03 |
Benchmark for program logistic_map
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
523.7 ± 5.3 | 517.0 | 532.5 | 1.00 |
cairo-native (embedded AOT) |
1838.6 ± 12.2 | 1820.4 | 1856.6 | 3.51 ± 0.04 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2000.1 ± 13.1 | 1984.7 | 2019.2 | 3.82 ± 0.05 |
363e838 to
8f28b8e
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1551 +/- ##
==========================================
- Coverage 72.88% 72.87% -0.02%
==========================================
Files 110 110
Lines 27137 27152 +15
==========================================
+ Hits 19780 19788 +8
- Misses 7357 7364 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Hey @TomerStarkware, a few things to address:
|
8f28b8e to
19712ec
Compare
TomerStarkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 and 2 Done,
How do I share the benchmark?
The second pointer existed before. Now you can access the metadata fields (ref_count, max_len) with a single pointer, whereas before you needed a double dereference for them.
@TomerStarkware made 1 comment.
Reviewable status: 0 of 6 files reviewed, all discussions resolved (waiting on @azteca1998, @edg-l, @entropidelic, @gabrielbosio, @igaray, @jrchatruc, @Oppen, and @pefontana).
TomerStarkware
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant, is there a command to run?
@TomerStarkware made 1 comment.
Reviewable status: 0 of 6 files reviewed, all discussions resolved (waiting on @azteca1998, @edg-l, @entropidelic, @gabrielbosio, @igaray, @jrchatruc, @Oppen, and @pefontana).

Title
Closes #NA
Introduces Breaking Changes?
No.
starknet-blocks.ymlworkflow to use these PRs.These PRs should be merged after this one right away, in that order.
Checklist
This change is