add run_with_libfunc_profile + AotWithProgram variant for ContractExecutor#1613
add run_with_libfunc_profile + AotWithProgram variant for ContractExecutor#1613avi-starkware wants to merge 2 commits into
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
60e4055 to
a21f3f5
Compare
orizi
left a comment
There was a problem hiding this comment.
@orizi reviewed all commit messages and made 1 comment.
Reviewable status: 1 of 4 files reviewed, 1 unresolved discussion (waiting on avi-starkware, TomerStarkware, and Yoni-Starkware).
src/executor.rs line 31 at r3 (raw file):
}; use bumpalo::Bump; // Re-exported so libfunc-profiling consumers (e.g. blockifier) can refer to the
what kind of reference is expected?
if it is just as a value - since you only use is as Arc<Program> - export:
pub type ArcProgram = std::sync::Arc<cairo_lang_sierra::program::Program>;
- 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>
aa4bf47 to
e6a4ad9
Compare
a21f3f5 to
be6cb3f
Compare
e6a4ad9 to
538f655
Compare
be6cb3f to
8b6515b
Compare
…cutor
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.
- metadata::profiler::Profile is now pub (was a private type alias).
- AotContractExecutor::run_with_libfunc_profile<H, F> (gated on
with-libfunc-profiling, in new file src/executor/libfunc_profile.rs)
wraps run with the bookkeeping the runtime needs:
1. Acquires a process-wide PROFILE_LOCK so concurrent profile calls
serialize on the global trace-id symbol. The lock is recovered
if poisoned.
2. Looks up the profile-id symbol before touching any global state.
Absent symbol -> typed Error::UnexpectedValue rather than panic.
3. Allocates a trace ID, inserts a slot in LIBFUNC_PROFILE, points
the global at the new ID.
4. Calls run; on success drains the slot and hands the Profile to
on_profile. On failure the callback is not invoked.
5. A ProfilerGuard restores the previous trace ID on success and
unwind.
- ContractExecutor::AotWithProgram(AotWithProgram { executor, program })
is a new variant that bundles an AOT executor with the Sierra program
it was built from. From<AotWithProgram> is provided.
- ContractExecutor::run dispatches the new variant via
run_with_libfunc_profile with a no-op profile callback.
- ContractExecutor::run_with_profile<H, F> is the profile-capturing
counterpart of run; for non-AotWithProgram variants it falls through
to run (callback never fires).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
538f655 to
c8ed1f7
Compare
8b6515b to
7f27488
Compare
…ion fixes Review-driven fixes to the ContractExecutor libfunc-profiling path: - ArcProgram alias (orizi review on src/executor.rs): replace the bare `pub use ...::Program` re-export with `pub type ArcProgram = Arc<Program>`, since profiling/sierra-emu consumers only ever hold the program as a shared handle. Gated on `any(sierra-emu, with-libfunc-profiling)` so EmuContractInfo consumers can name the field type too, and adopted for EmuContractInfo.program, AotWithProgram.program, and the run_with_profile callback. - Self-deadlock: run_with_libfunc_profile held the non-reentrant process-wide PROFILE_LOCK across self.run, which re-enters this function on the same thread whenever a profiled contract calls another contract -- a guaranteed hang. Take the lock only at the outermost profiled frame per thread (tracked via a thread-local PROFILE_DEPTH); nested frames inherit the outer frame's lock, and the existing per-call old_trace_id save/restore keeps the global trace-id symbol correct across nesting. Cross-thread isolation is preserved (the outermost frame holds the lock for the whole run tree). A ProfileDepthGuard decrements the depth on every exit path including unwind. - FFI panic-safety: ProfilerImpl::push_stmt is a `pub extern "C" fn` invoked directly from compiled Cairo code; its `.lock().unwrap()` would unwind across the C ABI on a poisoned mutex. Recover from poison instead, matching the lock handling in run_with_libfunc_profile. - Instrumented compiler binary: add a `with-libfunc-profiling` feature to starknet-native-compile forwarding to `cairo-native/with-libfunc-profiling`, so an instrumented binary (whose emitted .so carries the profiler symbol that run_with_libfunc_profile looks up) can be built. A binary built without it produces .so files with no profiler symbol, making every profiled call fail. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- 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.
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.Replaces #1599 / #1609.
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). AProfilerGuardrestores the previous trace ID and drops theLIBFUNC_PROFILEslot on both the success and unwind paths.ContractExecutor::AotWithProgram(AotWithProgram { executor, program })is a new variant that bundles an AOT executor with the Sierra program it was built from.From<AotWithProgram>is provided.ContractExecutor::rundispatches the new variant viarun_with_libfunc_profilewith a no-op profile callback.ContractExecutor::run_with_profile<H, F>is the profile-capturing counterpart ofrun; for non-AotWithProgramvariants it falls through torun(callback never fires).Stack
ContractExecutor(supersedes add ContractExecutor dispatch enum (Aot + Emu) #1598 / add ContractExecutor dispatch enum (Aot + Emu) #1608)run_with_libfunc_profile+AotWithProgram(supersedes add run_with_libfunc_profile + AotWithProgram variant for ContractExecutor #1599 / add run_with_libfunc_profile + AotWithProgram variant for ContractExecutor #1609)Test plan
cargo check(default features) cleancargo check --features with-libfunc-profilingcleancargo check --features sierra-emu,with-libfunc-profilingcleancargo check --workspace --all-featurescleanThis change is