Skip to content

perf: yield between SSE events - #1

Closed
daanishkhazi wants to merge 3 commits into
mainfrom
codex/cooperative-sse-processing
Closed

perf: yield between SSE events#1
daanishkhazi wants to merge 3 commits into
mainfrom
codex/cooperative-sse-processing

Conversation

@daanishkhazi

@daanishkhazi daanishkhazi commented Aug 14, 2026

Copy link
Copy Markdown

Summary

  • Replace repeated immutable SSE buffer concatenation with a reusable bytearray, clearing it before yielding each completed chunk.
  • Track cumulative synchronous JSON and response-assembly time across completed SSE events and cooperatively yield after 1 ms of actual processing.
  • Include malformed-event parsing in the same budget and exclude network waits, so naturally paced streams do not incur redundant scheduler hops.

This is intentionally narrow: it improves fairness between complete SSE events. It does not preempt parsing inside one giant individual JSON event.

Test plan

  • PYTHONPATH=src python -m pytest -q src/fireworks/training/sdk/tests/test_sampling.py src/fireworks/training/sdk/tests/test_deployment.py — 140 passed
  • Full training SDK suite — 593 passed, 23 skipped, 4 baseline failures reproduced on unmodified v1.2.9 because the borrowed environment has Tinker 0.22.3 rather than the repository's 0.23.0 pin, plus one pre-existing incorrect test import
  • Ruff check — passed
  • git diff --check — passed
  • CodeQL and Cubic review — passed

The single added regression splits inside the first SSE event, then delivers both completed events together in the second raw chunk. It proves the cooperative checkpoint runs between event decodes—not once per event—and preserves text, completion IDs, logprobs, routing metadata, performance metrics, and response closure. The same regression fails against unmodified v1.2.9.

Real rollout evidence

Braintrust: sdk-proof-1786752192-cd5fe5b5

A bounded deployment-only proof used this exact SDK head plus the Cookbook streaming transport with two DeepSeek rollout replicas and 128 authentic APEX episodes. Across 1,506 / 1,506 successful sampling calls, the SDK processed 1.27M SSE events / 1.90 GB and took 24,486 cooperative parser yields. Event-loop lag was p50 13 ms, p95 150 ms, and p99 228 ms, with no steady-state sampling, sandbox, MCP, or PTY failures.

The run was deliberately stopped after 481 seconds before any full long episode completed; it validates response-processing fairness, not a complete training step. All Fireworks and Modal resources were deleted.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 3 files

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/fireworks/training/sdk/sampling.py
Comment thread src/fireworks/training/sdk/tests/test_sampling.py Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 2 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread src/fireworks/training/sdk/sampling.py
Comment thread src/fireworks/training/sdk/tests/test_sampling.py Outdated
@daanishkhazi

Copy link
Copy Markdown
Author

Closing in favor of #2 rebased directly on v1.2.9. The controlled APEX ablation supports only the dedicated managed sampling loop; the parser-yield change is not required.

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