Skip to content

perf: avoid eager full stack materialization in sync trace source capture - #224

Merged
suchintan merged 4 commits into
mainfrom
repo-sync/rustwright-cloud-204
Aug 13, 2026
Merged

perf: avoid eager full stack materialization in sync trace source capture#224
suchintan merged 4 commits into
mainfrom
repo-sync/rustwright-cloud-204

Conversation

@suchintan

Copy link
Copy Markdown
Member

@suchintan suchintan added the sync Automated cross-repository sync label Aug 13, 2026
@claude

claude Bot commented Aug 13, 2026

Copy link
Copy Markdown

Summary

Solid, well-tested perf change: replaces inspect.stack()[2:] with a manual sys._getframe/f_back walk (_walk_source_stack) for sync trace source-stack capture, avoiding eager materialization of the full stack (including linecache source-context lookups) on every hooked action. Equivalence with the old eager path is verified both via mocked/adversarial cases and against the real call stack. No Rust/FFI/CDP code is touched by this PR.

🔴 Critical Issues (0)

None found.

🟡 Suggestions (2)
  • sys._getframe(2) failure mode differs from inspect.stack()[2:]. inspect.stack() walks frames via f_back and simply returns fewer entries (or an empty list) when the stack is shallower than requested; sys._getframe(2) raises ValueError: call stack is not deep enough if depth 2 doesn't exist (python/rustwright/sync_api.py new _record_source_stack, around the frame = sys._getframe(2) call). In real usage this should always resolve, since _record_source_stack is only ever called from internal wrapper code (never directly from user top-level code), so there are always ≥2 frames above it — but it's an unhandled, uncaught path if that assumption is ever violated (e.g. a future refactor that calls _record_source_stack one layer shallower, or direct use of the private method). Worth a defensive comment or an explicit try/except ValueError → skip-capture, matching the old "just capture nothing" fallback behavior, rather than relying implicitly on call-chain depth.
  • CHANGELOG.md [Unreleased] wasn't updated. CONTRIBUTING.md asks for a bullet under ## [Unreleased] for user-facing changes; this is a real user-visible perf improvement to tracing.start(sources=True) capture overhead and would be a natural one-liner there (mirroring the style of other "Changed" entries).
📝 Minor / Style (2)
  • The eager-reference implementation (_eager_reference) is duplicated near-verbatim across benchmarks/trace_stack_capture.py and tests/test_trace_stack_capture.py (~50 lines each). Understandable for keeping the benchmark/test scripts standalone, but a shared fixture/helper module would remove the risk of the two copies drifting apart over time.
  • Very minor semantic note: inspect.stack()'s .filename comes from inspect.getsourcefile() (falls back to frame.f_code.co_filename only when no on-disk source is found), while the new code always uses frame.f_code.co_filename directly. These are identical for ordinary on-disk .py files (the common case, and what the tests exercise), but could theoretically diverge for .pyc-only/frozen/zipimport modules. Not worth guarding against given Rustwright's usage patterns — just flagging so it's a conscious tradeoff rather than an overlooked one.

@suchintan
suchintan merged commit 38c7dfe into main Aug 13, 2026
15 of 16 checks passed
@suchintan
suchintan deleted the repo-sync/rustwright-cloud-204 branch August 13, 2026 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sync Automated cross-repository sync

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant