arena for dict#1616
Conversation
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 14 files and all commit messages, and made 1 comment.
Reviewable status: 14 of 16 files reviewed, 1 unresolved discussion (waiting on TomerStarkware).
src/executor.rs line 360 at r1 (raw file):
blake_call_count: u64, execution_arena: Bump, dict_registry: Vec<*mut FeltDict>,
probably not really needed.
add todo
995e422 to
655aa5d
Compare
TomerStarkware
left a comment
There was a problem hiding this comment.
@TomerStarkware made 1 comment.
Reviewable status: 14 of 20 files reviewed, 1 unresolved discussion (waiting on orizi).
src/executor.rs line 360 at r1 (raw file):
Previously, orizi wrote…
probably not really needed.
add todo
Done.
Benchmarking resultsBenchmark for program
|
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
11.251 ± 0.036 | 11.196 | 11.304 | 6.09 ± 0.08 |
cairo-native (embedded AOT) |
1.848 ± 0.023 | 1.815 | 1.875 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1.888 ± 0.018 | 1.862 | 1.922 | 1.02 ± 0.02 |
Benchmark for program dict_snapshot
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
555.0 ± 10.4 | 543.6 | 577.7 | 1.00 |
cairo-native (embedded AOT) |
1682.2 ± 35.3 | 1626.4 | 1745.4 | 3.03 ± 0.09 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1731.2 ± 10.8 | 1720.2 | 1756.7 | 3.12 ± 0.06 |
Benchmark for program factorial_2M
Open benchmarks
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
4.890 ± 0.104 | 4.802 | 5.102 | 2.27 ± 0.05 |
cairo-native (embedded AOT) |
2.153 ± 0.014 | 2.127 | 2.177 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
2.198 ± 0.020 | 2.179 | 2.237 | 1.02 ± 0.01 |
Benchmark for program fib_2M
Open benchmarks
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
4.831 ± 0.073 | 4.756 | 4.949 | 2.91 ± 0.06 |
cairo-native (embedded AOT) |
1.660 ± 0.022 | 1.630 | 1.689 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1.705 ± 0.025 | 1.676 | 1.760 | 1.03 ± 0.02 |
Benchmark for program linear_search
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
591.3 ± 8.4 | 579.3 | 607.9 | 1.00 |
cairo-native (embedded AOT) |
1692.0 ± 17.5 | 1666.7 | 1712.5 | 2.86 ± 0.05 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1784.7 ± 21.6 | 1765.8 | 1837.7 | 3.02 ± 0.06 |
Benchmark for program logistic_map
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
500.8 ± 4.1 | 495.8 | 509.2 | 1.00 |
cairo-native (embedded AOT) |
1835.6 ± 14.5 | 1798.1 | 1849.7 | 3.67 ± 0.04 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1999.3 ± 16.2 | 1977.8 | 2034.4 | 3.99 ± 0.05 |
Benchmark results Main vs HEAD.Base
Head
Base
Head
Base
Head
Base
Head
Base
Head
Base
Head
|
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed all commit messages and made 1 comment.
Reviewable status: 12 of 20 files reviewed, 2 unresolved discussions (waiting on TomerStarkware).
-- commits line 4 at r2:
rebase
655aa5d to
c57e4f9
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 8 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on TomerStarkware).
src/executor.rs line 360 at r1 (raw file):
Previously, TomerStarkware wrote…
Done.
add the why
c57e4f9 to
af42e6f
Compare
af42e6f to
f97c196
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 1 file and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on TomerStarkware).
Move felt252_dict storage to the per-execution arena
Closes #NA
Moves
FeltDictallocation off the system heap and onto the per-executionBumparena, completing the arena migration for boxed values, nullables,arrays, and dictionaries.
Before, each dict was
Rc::into_raw(Rc::new(FeltDict { ... }))with its element buffer on the system heap (alloc/realloc/dealloc) and dup/dropwired through runtime bindings + per-value drop overrides materialized via
Felt252DictOverrides+ symbol lookup. Now:cairo_native__dict_newarena-allocates theFeltDictstruct and registers it in a thread-localDICT_REGISTRY.cairo_native__dict_getgrows the elements buffer viaEXECUTION_ARENA.alloc_layout+copy_nonoverlappinginstead ofrealloc.InvocationGuard::drop, the registry is drained and eachFeltDict'sHashMapisdrop_in_place'd (the HashMap is the only system-heapallocation that survives in the FeltDict). The arena is then reset, freeing the
FeltDictstruct and elements buffer in one shot.This removes:
src/metadata/felt252_dict.rs(Felt252DictOverrides).cairo_native__dict_drop/cairo_native__dict_dupruntime bindings.Drop for FeltDict.find_dict_drop_overrideclosure threaded through everyAbiArgument::to_bytesimpl and throughValue::to_ptr.Net diff: -894 / +262.
Introduces Breaking Changes?
Yes.
AotNativeExecutor::newandJitNativeExecutor::newno longer take aFelt252DictOverridesargument.AbiArgument::to_bytesno longer takes thefind_dict_drop_overrideclosure.Value::to_ptrno longer takes thefind_dict_drop_overrideclosure.FeltDictno longer has adrop_fnfield, no longer implsDrop, andcairo_native__dict_newno longer takes a drop callback.cairo_native__dict_dropandcairo_native__dict_dupsymbols are removed.This change is