feat: make trace collection easy to disable (docs + ERYX_PROFILE_TRACE) - #424
Merged
Merged
Conversation
`SandboxBuilder` defaults to `collect_trace: true`, which installs `sys.settrace` for every execution. The hook's cost scales with the amount of Python run: cold `pass` 1.28 ms vs 1.08 ms, a small json + string.Template render 3.7 ms vs 0.9 ms, and `sum(i * i for i in range(20_000))` 369 ms vs 3.8 ms (~96x). The knob to turn it off (`with_trace_collection(false)`, #270) existed but was not discoverable and the profiling harness always ran with it on. - Document the cost and the session/handler interaction on `SandboxBuilder::with_trace_collection`. - Add an `ERYX_PROFILE_TRACE=0` env switch to `profile_execution.rs`, `session_bench.rs` and the criterion bench's `create_sandbox()`. Default (unset) keeps tracing on so existing numbers stay comparable. - Explain in the pyeryx `Sandbox` docstring / stubs that Python sandboxes never collect traces (hardcoded off since #270 because `ExecuteResult` has no trace field), and comment the two call sites. - Add a "Trace Collection" subsection to the sandboxes guide. No defaults change and no new API is added. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Contributor
|
🌐 Demo preview: https://feat-trace-collection-knob.eryx-bvy.pages.dev |
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
SandboxBuilderdefaults tocollect_trace: true, which installs Python'ssys.settracehook for every execution. The hook fires on every line/call/return, so its cost scales with the amount of Python executed and it dominates anything heavier thanpass. Measured 2026-09-10 (stock wasmtime 48.0.1, Ryzen 9 7950X, fresh instance per call):passimport json, string; d = json.loads(...); string.Template(...).substitute(...))total = sum(i * i for i in range(20_000))The knob to turn it off already exists (
SandboxBuilder::with_trace_collection(false), added in #270), but nothing pointed at it: the doc comment did not quantify the cost, the profiling harness and criterion bench always ran with it on, and the book never mentioned it. This PR makes the knob discoverable and adds an env switch for measurement. It does not change any default, so existing numbers stay comparable.What changed
Rust API (
crates/eryx/src/sandbox.rs):with_trace_collectiondoc comment now carries the cost table above, an example, and notes thatInProcessSessions created from the sandbox inherit the setting while aTraceHandlerkeeps tracing on regardless. No new API:with_trace_collectionalready covers sandboxes and in-process sessions, andSessionExecutornever installssys.settraceunlesswith_tracing(tx)is called explicitly.Harness / benches:
ERYX_PROFILE_TRACE=0disables trace collection (same semantics as the knob in the closed perf(eryx): keep the instance heap mapped with glibc malloc tunables #419: any other value or unset keeps the default) incrates/eryx/examples/profile_execution.rscrates/eryx/examples/session_bench.rscrates/eryx/benches/execution.rs(create_sandbox()helper, so every criterion group honours it)If
perf/instantiation-overhead(perf(eryx): pool instances and pre-instantiate stores for stateless execution #411) merges, itsprofile_stateless.rsshould get the same three lines.Python: no API change, deliberately.
eryx.Sandbox,SandboxFactory.create_sandboxand (viaSessionExecutor)Sessionhave had trace collection hardcoded off since Optimize callback setup and Python tracing #270, and pyeryx'sExecuteResultdoes not expose atracefield, so acollect_trace=Trueoption would be a pure performance regression that exposes nothing. Instead theSandboxdocstring /_eryx.pyinow say so explicitly, and the twowith_trace_collection(false)sites carry a comment explaining why.Book (
book/src/guide/sandboxes.md): new "Trace Collection" subsection under Execution Results with the cost and how to disable it.How to disable tracing
# Profiling harness / criterion bench: ERYX_PROFILE_TRACE=0 cargo run --release --example profile_execution --features embedded ERYX_PROFILE_TRACE=0 cargo bench --package eryx --features embeddedTests
test_trace_collection_can_be_disabled/test_trace_handler_without_result_collectionincrates/eryx/tests/trace_events_precise.rsalready cover the Rust behaviour; no new test was needed. No Python test was added because there is no Python-visible behaviour to assert (no option, notracefield).cargo nextest run --workspace --features embedded --cargo-profile release: 624 passed, 0 failedcargo clippy --workspace --all-targets --all-features -- -D warnings: cleancargo fmt --all --check: cleanmise run build+uv run pytest tests/ -q: 333 passed, 0 failed🤖 Generated with Claude Code