Skip to content

Python: perf(foundry-hosting): parallelize pre-model storage reads - #8363

Merged
Evan Mattson (moonbox3) merged 4 commits into
microsoft:mainfrom
harsheet-shah:harsheetshah/foundry-hosting-parallel-reads-v2
Sep 15, 2026
Merged

Evan Mattson (moonbox3) merged 4 commits into
microsoft:mainfrom
harsheet-shah:harsheetshah/foundry-hosting-parallel-reads-v2

Conversation

@harsheet-shah

@harsheet-shah Harsheet Shah (harsheet-shah) commented Sep 14, 2026 •

Copy link
Copy Markdown
Contributor

Closes #8365

What

Parallelize the three pre-model storage reads in the foundry_hosting non-workflow response handler (ResponsesHostServer._handle_inner_agent). Previously the caller's input items, the prior conversation history, and the agent session were loaded with three strictly-sequential awaits before the model call. They are independent storage round-trips with no data dependency, so on warm / multi-turn requests they added avoidable serial latency to the request critical path.

Change

  • New _load_request_messages helper gathers the input-items and history reads with asyncio.gather.
  • It is kicked off via asyncio.ensure_future right after the stores are built, so it overlaps the session load instead of running after it.
  • Result ordering (history precedes input) and all existing error semantics are preserved; the read task is cancelled if session preparation fails.
  • Stateless single-turn requests are unaffected — history and session loads short-circuit without a round-trip.

Why

A latency waterfall of the warm baseline showed the pre-model reads as a chain of serialized private-endpoint round-trips (~100-260ms each). Overlapping the independent ones removes serial wait on warm / multi-turn requests with no behavior change.

Testing

  • Syntax/AST validated; overlay onto the released package import-verified.
  • End-to-end benchmarked on a hosted Foundry agent (uksouth), stock baseline vs this change, isolated by overlaying only the patched `_responses.py` onto the :stock image (byte-identical baseline otherwise).

Ran a multi-turn load of 500 cycles at concurrency 50, where turn-2 is the measured request (it carries previous_response_id, so it loads both the session and the prior history — the path this PR changes). At the median (p50), turn-2 time-to-last-byte improved slightly from 2,803 ms on the stock baseline to 2,799 ms with this change (about 4 ms faster). At the 99th percentile (p99), it improved from 5,305 ms to 4,902 ms — roughly 403 ms, or 7.6%, faster.

turn-2 TTLB Baseline (stock) This PR Δ
p50 2,803 ms 2,799 ms -4 ms (-0.1%)
p99 5,305 ms 4,902 ms -403 ms (-7.6%)

The non-workflow response handler loaded the caller's input items, the prior
conversation history, and the agent session with three strictly-sequential
awaits before the model call. These are independent storage round-trips with
no data dependency, so on warm/multi-turn requests they added avoidable serial
latency to the request critical path.

Load them concurrently: the input/history reads are gathered in a new
_load_request_messages helper and kicked off (via ensure_future) so they
overlap the session load. Result ordering (history precedes input) and the
existing error semantics are preserved; the read task is cancelled if session
preparation fails. Stateless single-turn requests are unaffected (history and
session loads short-circuit without a round-trip).

Co-authored-by: Copilot <[email protected]>
Copilot-Session: 3d900395-13d0-4698-bf9d-f6670f9e545c

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Changes recommended

Task cleanup is incomplete on failures and cancellation, allowing orphaned storage operations.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Parallelizes pre-model storage reads to reduce Foundry-hosted agent latency.

Changes:

  • Concurrently loads input messages and conversation history.
  • Overlaps message loading with session retrieval.
  • Attempts to cancel message loading when session preparation fails.
File summaries
File Description
_responses.py Adds concurrent request-message loading and session-read overlap.
Review details

Suppressed comments (1)

python/packages/foundry_hosting/agent_framework_foundry_hosting/_responses.py:792

  • This cleanup misses CancelledError and GeneratorExit because they are not Exception. If the request is cancelled or the stream is closed during session_storage.get, the newly spawned message-loading task keeps running. Even for an ordinary session error, calling cancel() without awaiting the task returns before its cleanup finishes. Catch BaseException, cancel and drain the task, and retain logging only for ordinary exceptions.
            if request_messages_task is not None:
                request_messages_task.cancel()
  • Files reviewed: 1/1 changed files
  • Comments generated: 2
  • Review effort level: Balanced (auto)

Note

Copilot is running an experiment and ran this review at Balanced.


💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

- _load_request_messages now owns both read tasks and, on any BaseException
  (including CancelledError/GeneratorExit from gather's first-failure or an
  outer cancellation), cancels and drains the sibling so no storage read is
  orphaned.
- The helper returns the assembled [*history, *input] list, so the message
  ordering rule lives only in the helper (per review); the caller no longer
  reverses the tuple.
- Session-prep cleanup now catches BaseException, cancels AND awaits (drains)
  the in-flight read task, and logs only ordinary Exceptions.
- Add TestParallelRequestReads: overlap, message ordering, history-skip when
  not agent-server history, and sibling-cancellation on a failed read.

Co-authored-by: Copilot <[email protected]>
Copilot-Session: 3d900395-13d0-4698-bf9d-f6670f9e545c
@harsheet-shah

Copy link
Copy Markdown
Contributor Author

Thanks for the review — addressed in b27b81b:

  • Orphaned sibling read on first failure (Copilot): _load_request_messages now owns both reads as explicit tasks. On any BaseException from the gather (its first-failure semantics don't cancel the sibling) — or from an outer cancellation — it cancels both tasks and awaits them with return_exceptions=True to drain, then re-raises. No storage read is left running.
  • Session-prep except missed CancelledError/GeneratorExit (Copilot): that handler now catches BaseException, cancels and drains the in-flight message task (with suppress(BaseException): await task), and logs only ordinary Exceptions.
  • Assemble in the helper (moonbox3): the helper now returns the assembled [*history, *input] list, so the ordering rule lives in one place; the caller just uses messages = await request_messages_task.
  • Regression tests (Copilot): added TestParallelRequestReads covering read overlap (event-gated — both in flight before either completes), the [history, input] ordering, history-skip when not agent-server-history, and sibling cancellation/drain when one read fails.

Note: my local env can't reach PyPI, so the tests were validated via CI rather than locally.

@eavanvalkenburg

Copy link
Copy Markdown
Member

Thanks for the update. Before this is ready, could you please:

Once those are addressed, please re-request review. Thanks!

…eads

Adds a handler-level regression test proving that when session
preparation fails, the concurrently-launched input/history read task
started by _handle_inner_agent is cancelled and drained rather than
orphaned. Completes the no-orphan coverage requested in review
(the helper-level read-failure path was already covered).

Co-authored-by: Copilot <[email protected]>
Copilot-Session: 3d900395-13d0-4698-bf9d-f6670f9e545c
@moonbox3
Evan Mattson (moonbox3) added this pull request to the merge queue Sep 15, 2026
Merged via the queue into microsoft:main with commit 8236fe0 Sep 15, 2026
40 checks passed

This branch was previously deployed

1 inactive deployment
github-app-auth — 439f4f8c Deployed Sep 15, 2026 by moonbox3 via add_label #22858
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Usage: [Issues, PRs], Target: Python

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf(foundry-hosting): pre-model storage reads run sequentially in ResponsesHostServer._handle_inner_agent

4 participants