add run_with_libfunc_profile + AotWithProgram pairing#1613
Conversation
|
✅ Code is now correctly formatted. |
98ed085 to
64c12b3
Compare
474462f to
13381fa
Compare
64c12b3 to
7e5fcd7
Compare
b5e8d6b to
94bc246
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 3 files and all commit messages.
Reviewable status: 3 of 4 files reviewed, all discussions resolved (waiting on TomerStarkware and Yoni-Starkware).
7e5fcd7 to
4176352
Compare
94bc246 to
24984b5
Compare
4176352 to
7af0508
Compare
24984b5 to
78bf3e4
Compare
7af0508 to
38c5f2f
Compare
78bf3e4 to
5b13906
Compare
38c5f2f to
81093b3
Compare
5b13906 to
de234b8
Compare
81093b3 to
4f53f27
Compare
74aab36 to
d19f2ac
Compare
- Drop unused `NativeCompiledClassV1::new_from_emu`; it had no in-tree callers, and the sierra-emu construction path will be re-added when the benchmarking/replay tool that needs it lands. - Revert `process_compilation_request`'s `.map(...)` shape to a direct `match`, moving `casm` into `NativeCompiledClassV1::new(_with_program)` in the `Ok` arm at zero cost instead of cloning it. - Bump pinned cairo-native rev to pick up `cargo fmt` fix on starkware-libs/cairo_native#1613. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
4f53f27 to
6f77130
Compare
d19f2ac to
60e4055
Compare
6f77130 to
aa4bf47
Compare
- Drop unused `NativeCompiledClassV1::new_from_emu`; it had no in-tree callers, and the sierra-emu construction path will be re-added when the benchmarking/replay tool that needs it lands. - Revert `process_compilation_request`'s `.map(...)` shape to a direct `match`, moving `casm` into `NativeCompiledClassV1::new(_with_program)` in the `Ok` arm at zero cost instead of cloning it. - Bump pinned cairo-native rev to pick up `cargo fmt` fix on starkware-libs/cairo_native#1613. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
avi-starkware
left a comment
There was a problem hiding this comment.
@avi-starkware made 1 comment.
Reviewable status: 0 of 5 files reviewed, 1 unresolved discussion (waiting on orizi, TomerStarkware, and Yoni-Starkware).
src/executor.rs line 31 at r3 (raw file):
Previously, orizi wrote…
what kind of reference is expected?
if it is just as a value - since you only use is asArc<Program>- export:
pub type ArcProgram = std::sync::Arc<cairo_lang_sierra::program::Program>;
Done.
0b0a902 to
6640ce9
Compare
c8ed1f7 to
90cd01f
Compare
6640ce9 to
c7e5bcc
Compare
90cd01f to
a1e3635
Compare
c7e5bcc to
7107710
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 5 files and all commit messages, made 1 comment, and resolved 1 discussion.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on avi-starkware, TomerStarkware, and Yoni-Starkware).
src/executor/contract_executor.rs line 150 at r5 (raw file):
/// callers don't have to keep their own copy around just to resolve libfunc samples. /// For other variants this is identical to `run` and `on_profile` is never invoked. #[cfg(feature = "with-libfunc-profiling")]
following my response to the previous PR - seems to me this is basically the only interesting function.
0f1975e to
6184f7a
Compare
- Drop unused `NativeCompiledClassV1::new_from_emu`; it had no in-tree callers, and the sierra-emu construction path will be re-added when the benchmarking/replay tool that needs it lands. - Revert `process_compilation_request`'s `.map(...)` shape to a direct `match`, moving `casm` into `NativeCompiledClassV1::new(_with_program)` in the `Ok` arm at zero cost instead of cloning it. - Bump pinned cairo-native rev to pick up `cargo fmt` fix on starkware-libs/cairo_native#1613. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Previously, orizi wrote…
Following my response to your response - removed this file |
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed 3 files and all commit messages, and resolved 1 discussion.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on TomerStarkware and Yoni-Starkware).
orizi
left a comment
There was a problem hiding this comment.
@orizi made 1 comment.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on TomerStarkware and Yoni-Starkware).
4fdd841 to
f62b57a
Compare
6184f7a to
a697f57
Compare
a697f57 to
d17afb3
Compare
Benchmarking resultsBenchmark for program
|
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
10.461 ± 0.234 | 10.240 | 10.848 | 5.88 ± 0.14 |
cairo-native (embedded AOT) |
1.780 ± 0.010 | 1.767 | 1.795 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1.799 ± 0.014 | 1.781 | 1.833 | 1.01 ± 0.01 |
Benchmark for program dict_snapshot
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
527.9 ± 3.1 | 523.7 | 534.8 | 1.00 |
cairo-native (embedded AOT) |
1592.8 ± 5.8 | 1581.4 | 1599.5 | 3.02 ± 0.02 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1628.2 ± 11.8 | 1618.5 | 1653.9 | 3.08 ± 0.03 |
Benchmark for program factorial_2M
Open benchmarks
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
4.512 ± 0.026 | 4.484 | 4.554 | 2.66 ± 0.04 |
cairo-native (embedded AOT) |
1.698 ± 0.021 | 1.658 | 1.724 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1.722 ± 0.014 | 1.694 | 1.741 | 1.01 ± 0.02 |
Benchmark for program fib_2M
Open benchmarks
| Command | Mean [s] | Min [s] | Max [s] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
4.430 ± 0.021 | 4.399 | 4.464 | 2.76 ± 0.02 |
cairo-native (embedded AOT) |
1.603 ± 0.011 | 1.587 | 1.628 | 1.00 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1.629 ± 0.009 | 1.617 | 1.643 | 1.02 ± 0.01 |
Benchmark for program linear_search
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
569.2 ± 4.5 | 564.7 | 579.5 | 1.00 |
cairo-native (embedded AOT) |
1622.1 ± 5.5 | 1614.1 | 1631.9 | 2.85 ± 0.02 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1671.1 ± 29.1 | 1649.6 | 1737.7 | 2.94 ± 0.06 |
Benchmark for program logistic_map
Open benchmarks
| Command | Mean [ms] | Min [ms] | Max [ms] | Relative |
|---|---|---|---|---|
Cairo-vm (Rust, Cairo 1) |
486.6 ± 4.7 | 481.4 | 498.4 | 1.00 |
cairo-native (embedded AOT) |
1608.5 ± 6.6 | 1601.2 | 1626.1 | 3.31 ± 0.03 |
cairo-native (embedded JIT using LLVM's ORC Engine) |
1633.6 ± 6.2 | 1626.3 | 1647.2 | 3.36 ± 0.04 |
Benchmark results Main vs HEAD.Base
Head
Base
Head
Base
Head
Base
Head
Base
Head
Base
Head
|
AotContractExecutor::run_with_libfunc_profile wraps run() with the per-call trace-id and lock bookkeeping the with-libfunc-profiling runtime needs, draining the collected Profile to a callback on success. AotWithProgram pairs a compiled executor with the Sierra program it was built from and exposes run()/run_with_profile() in the same shape as AotContractExecutor::run, so callers can swap between them without changing call sites. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
d17afb3 to
1990910
Compare
avi-starkware
left a comment
There was a problem hiding this comment.
@avi-starkware reviewed all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on TomerStarkware and Yoni-Starkware).
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed all commit messages.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on TomerStarkware and Yoni-Starkware).
Summary
Exposes the libfunc-profiling primitives that downstream consumers (e.g. the blockifier in
starkware-libs/sequencer) currently maintain locally. The profile-collection pattern is callback-driven so the per-call key (tx hash, etc.) stays out of cairo-native.Follows the no-enum design from #1612 (merged): there is no
ContractExecutordispatch enum; consumers pick an executor type per feature-flag build.Changes
metadata::profiler::Profileis nowpub(was a private type alias).AotContractExecutor::run_with_libfunc_profile<H, F>(gated onwith-libfunc-profiling, in new filesrc/executor/libfunc_profile.rs) wrapsrun: allocates a unique trace ID, points the executor'scairo_native__profiler__profile_idsymbol at it, drains the resultingProfileafterrunreturns, and hands it to a caller-suppliedFnOnce(Profile)on success. AProfilerGuardrestores the previous trace ID and drops theLIBFUNC_PROFILEslot on both the success and unwind paths; a process-wide lock plus a thread-local depth counter make concurrent and nested (contract-calls-contract) profiled runs safe.AotWithProgram { executor, program }: pairs an AOT executor with the Sierra program it was compiled from (needed to resolve libfunc samples). Exposes inherent methods:run(...)— mirrorsAotContractExecutor::run; routes through the profiling bookkeeping (the instrumented.soneeds a live profile slot) and discards the profile.run_with_profile(..., FnOnce(Profile, ArcProgram))— hands the captured profile and the paired program to the callback.pub type ArcProgram = Arc<Program>re-export so consumers can name the program handle without a directcairo-lang-sierradependency.Stack
EmuContractExecutor(merged)run_with_libfunc_profile+AotWithProgramTest plan
cargo check(default features) cleancargo check --features with-libfunc-profilingcleancargo check --features sierra-emu,with-libfunc-profilingcleancargo check --workspace --all-featurescleanThis change is