Skip to content

Optimize slog dispatch for spawned cache tasks#350

Open
proboscis wants to merge 3 commits into
mainfrom
issue/slog-handler-perf-concurrent-spawn/run-20260318-114128
Open

Optimize slog dispatch for spawned cache tasks#350
proboscis wants to merge 3 commits into
mainfrom
issue/slog-handler-perf-concurrent-spawn/run-20260318-114128

Conversation

@proboscis

Copy link
Copy Markdown
Owner

Summary

  • add a WriterHandler fast-path for PyTell dispatch so cache slog() calls can write directly into the Rust store without re-entering the full handler path
  • cache handler trace metadata on prompt segments and reuse it during dispatch/trace assembly instead of recomputing debug_info() on every scan
  • dedupe the shared visible handler prefix when resuming unstarted continuations created from GetHandlers()/Spawn, which removes the duplicated handler chain seen in cached spawned tasks
  • add a regression test that compares the handler stack seen by a cached direct call vs the same cached call through Spawn

Verification

  • uvx maturin develop --release
  • uv run pytest tests/test_cache_await_spawn_hang.py -q
    • 7 passed in 5.52s
  • uv run pytest tests/test_cache_await_spawn_hang.py tests/test_with_handler_type_filter_regression.py tests/effects/test_local_handler_ask.py::test_local_eval_keeps_typed_handler_filtering -q
    • 20 passed in 6.65s
  • cache+slog benchmark (N=500, persistent sqlite cache, same cache task with cache slog monkeypatched on/off)
    • cache_slog_off ok= True elapsed= 1.318 len= 500
    • cache_slog_on ok= True elapsed= 2.089 len= 500
  • spawned cached-task handler probe
    • GetHandlers() length inside the spawned cached task is now 9 instead of the duplicated 18
    • direct cached call and spawned cached call now report the same handler stack

Notes

  • cargo test in packages/doeff-vm-core passed.
  • cargo test in packages/doeff-core-effects is currently blocked by pre-existing scheduler test compile errors that still reference ReadySet::Fifo/ReadySet::Priority; this is unrelated to the writer-dispatch fix.

Issue: slog-handler-perf-concurrent-spawn

@proboscis

Copy link
Copy Markdown
Owner Author

Addressed review round 1:

  • replaced WriterHandler string matching with a typed builtin discriminant (BuiltinHandlerKind::Writer)
  • replaced PyTell name matching with an isinstance(doeff_vm.PyTell) check backed by a cached Python type object
  • guarded unstarted-continuation dedupe with if !handlers.is_empty() and documented that prefix trimming is strictly position-sensitive
  • added an explicit comment that fast-path type-filter misses must fall back to full dispatch semantics
  • made the fast-path effect object conversion lazy so the common no-type-filter path avoids the extra Python conversion

Re-verified after the follow-up fix:

  • uvx maturin develop --release
  • uv run pytest tests/test_cache_await_spawn_hang.py tests/test_with_handler_type_filter_regression.py tests/effects/test_local_handler_ask.py::test_local_eval_keeps_typed_handler_filtering -q -> 21 passed in 7.36s
  • cargo test in packages/doeff-vm-core -> pass
  • benchmark: cache_slog_off 1.41s, cache_slog_on 2.162s for N=500

@proboscis

Copy link
Copy Markdown
Owner Author

Round 3 addressed in 0ac9ceb.

Changes:

  • removed the writer fast-path entirely (including the builtin writer discriminant plumbing)
  • removed the Arc::ptr_eq shared-prefix dedup
  • kept GetHandlers() full-stack semantics, but changed spawned continuation capture to store an inherited handler-prefix descriptor instead of rebasing resume onto a different segment
  • for SpawnAwaitHandlers, CreateContinuation now records the inherited visible handler prefix and an offset so prepended local handlers like the blocking sync_await_handler remain local instead of being skipped

Why this fixes the root cause:

  • the continuation still resumes relative to the actual resume site, so return/delegate topology stays intact
  • only the inherited portion of the captured handler list is elided at resume, and only after the local-prefix offset, so we no longer duplicate the caller-visible chain or accidentally drop task-local handlers

Validation:

  • uvx maturin develop --release
  • uv run pytest tests/test_cache_await_spawn_hang.py -q -> 7 passed
  • uv run pytest tests/test_cache_await_spawn_hang.py tests/test_with_handler_type_filter_regression.py tests/effects/test_local_handler_ask.py::test_local_eval_keeps_typed_handler_filtering -q -> 21 passed
  • cargo test in packages/doeff-vm-core -> pass
  • benchmark rerun after removing the writer fast-path (N=500):
    • cache_slog_off ok=True elapsed=2.400 len=500
    • cache_slog_on ok=True elapsed=2.456 len=500

proboscis added a commit that referenced this pull request Mar 18, 2026
Introduces a unified scope abstraction for the doeff VM that separates
static scope (handlers, interceptors, variables) from dynamic execution
state (segments, frames). Key changes:

- Scope as first-class concept: handler chain + interceptors + variable table
- Scoped variables with Python-like shadow/nonlocal semantics
- GetScopeOf(k) + CreateContinuation(program, scope) replaces GetHandlers
- ResumeContinuation becomes trivial (no handler re-installation)
- Fixes handler duplication in Spawn structurally
- RustStore/ScopeStore replaced by generic variable table
- TDD test plan with 24 tests

Motivated by: handler duplication bug (#342, PR #350), RustStore violation,
ScopeStore leaking into VM segments.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant