Skip to content

perf(flows): remove redundant event scans in prompt assembly - #6540

Open
dexhunter wants to merge 1 commit into
google:mainfrom
dexhunter:perf/reduce-prompt-assembly-passes
Open

perf(flows): remove redundant event scans in prompt assembly#6540
dexhunter wants to merge 1 commit into
google:mainfrom
dexhunter:perf/reduce-prompt-assembly-passes

Conversation

@dexhunter

@dexhunter dexhunter commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Link to Issue or Description of Change

No existing issue, so this description follows the issue-template structure.

Problem:

_get_contents runs once per LLM request over the whole session history, and it walks each event's parts several times.

The include/exclude filter shows this most plainly. _should_include_event_in_context calls _contains_empty_content, _is_adk_framework_event, _is_auth_event and _is_request_confirmation_event in sequence. Each one restarts the walk from the first part, so a two-part event gets scanned up to five times to answer one question. Separately, get_function_responses() and get_function_calls() rebuild a fresh list by rescanning every part on each call, and _rearrange_events_for_async_function_responses_in_history calls them repeatedly for the same event. None of that repeated work depends on anything that changes between the calls.

The cost lands on every request and scales with conversation length, so it grows quadratically over a long session.

Solution:

Remove the repetition without changing what gets assembled:

  • _should_include_event_in_context derives the two facts it needs (whether any part is visible, and whether the event is an internal ADK event) in a single pass over the parts, then applies the same boolean expression with the same operand order and short-circuit structure as before.
  • _rearrange_events_for_async_function_responses_in_history computes the function-call and function-response lists once per event and reuses them.
  • The compaction probe becomes an early-exit loop, and the loop-invariant strip_client_function_call_ids branch moves out of the per-part copy in _copy_content_for_request.

The change keeps every existing helper and every docstring. _contains_empty_content, _is_part_invisible and the three internal-event predicates still hold the readable definition of the contract. The rendered prompt does not change.

Measurements

Session history of 501 events (125 tool-using turns with function_call/function_response payloads). CPU time (time.process_time) for one _get_contents call. Each repetition builds fresh fixtures, and the collector runs before each timed call and stays off inside it. I measured baseline and change alternately within one window over 14 paired repetitions, so host drift hits both arms equally. This machine is shared, so the paired delta carries the claim rather than either absolute number:

CPU ms per _get_contents call
baseline (c12a025) 4.8001
this change 4.0470
paired delta −15.47% median (range −22.48% to −9.83%)

All 14 paired repetitions came out negative. That is −0.75 ms of framework CPU per LLM request at this history length.

What this is and is not: framework CPU per request, not user-visible latency. _get_contents is a synchronous function on the async request path, so the honest framing is the throughput and concurrency ceiling rather than single-request wall time. The figure is also specific to a long tool-using history; on a short history the absolute saving is proportionally smaller. I left out the larger item on purpose: roughly 60% of the remaining time is the per-Part and per-Content shallow copy that gives downstream processors session isolation. Removing that copy is worth more than this whole change, but it means changing the isolation contract 400f512d established, which is your call rather than something to slip into a perf PR.

The optimization search that produced the idea is recorded at https://dashboard.weco.ai/share/HBCrQEvgNK-VH8KnBeTtcIV_-h1NFbi0 as a record of the search only. I wrote and reviewed the diff in this PR by hand. I did not use the search's own best candidate, because it dissolved several helpers and diverged from upstream behaviour on the edge cases the tests below now cover.

Testing Plan

Unit Tests:

  • I have added or updated unit tests for my change.
  • All unit tests pass locally.

Added four regression tests pinning the filter contracts this refactor has to preserve. Each one fails against a version of the filter that gets the corresponding edge case wrong:

  • test_internal_events_excluded_even_with_transcription: an internal ADK event stays excluded whatever else it carries, since the exclusion does not depend on the emptiness check (parametrized over adk_framework, adk_request_credential, adk_request_confirmation).
  • test_internal_event_with_empty_role_and_transcription_excluded: the same, for the combination where a transcription keeps the event non-empty while an empty role keeps the parts from counting as visible.
  • test_compaction_event_included_but_still_branch_filtered: a compaction event is not empty, but branch filtering still applies to it.
  • test_content_with_parts_but_empty_role_is_empty: emptiness depends on the role as well as the parts.
$ pytest tests/unittests/flows/llm_flows/test_contents.py \
         tests/unittests/flows/llm_flows/test_contents_function.py \
         tests/unittests/flows/llm_flows/test_contents_branch.py \
         tests/unittests/flows/llm_flows/test_contents_other_agent.py
63 passed

$ pytest tests/unittests/flows/
484 passed

Beyond the tests, I compared prompt output byte-for-byte against the unmodified code across randomized histories under four argument combinations (default, preserve_function_call_ids=True, include_thoughts_from_other_agents=True, and a foreign agent_name), plus a fixture built to reach the awkward shapes: compaction events both plain and simultaneously framework/auth/confirmation, empty-role contents, thought-only parts, transcription-only events, branch scoping and cross-agent replies. The rendered contents are identical in every case.

Manual End-to-End (E2E) Tests:

This is a pure refactor of prompt assembly with byte-identical output, so the behavioural evidence is the byte-for-byte comparison above rather than a console transcript. To reproduce the measurement, the repo's own harness covers the same function:

uv run python tests/benchmarks/benchmark_contents.py

Note that tests/unittests/cli/utils/test_cli_tools_click.py::test_telemetry_cli_commands fails on c12a025 both with and without this change. It is unrelated and pre-existing.

Checklist

  • I have read the CONTRIBUTING.md document.
  • I have performed a self-review of my own code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have manually tested my changes end-to-end.
  • Any dependent changes have been merged and published in downstream modules.

_get_contents walks every event's parts several times per LLM request. The
include/exclude filter alone calls _contains_empty_content and the three
internal-event predicates in sequence, and each of those restarts the walk from
the first part. get_function_responses() and get_function_calls() rebuild a
fresh list on every call and the async rearrangement calls them repeatedly for
the same event. None of that work depends on anything that changes between the
calls.

This removes the repetition without changing what is assembled:

- _should_include_event_in_context derives the two facts it needs (whether any
  part is visible, and whether the event is an internal ADK event) in a single
  pass, then applies the same boolean expression with the same operand order
  and short-circuit structure.
- _rearrange_events_for_async_function_responses_in_history computes the
  function-call and function-response lists once per event and reuses them.
- The compaction probe folds into an early-exit loop, and the loop-invariant
  strip_client_function_call_ids branch is hoisted out of the per-part copy.

Every helper is kept and the rendered prompt is unchanged.

Measured on a 501-event tool-using history (125 turns, function_call and
function_response payloads), CPU time for one _get_contents call, baseline and
change alternating within one measurement window over 14 paired repetitions:

  baseline   4.8001 ms
  change     4.0470 ms
  paired delta  -15.47% median, -22.48% to -9.83%, all 14 repetitions negative

Adds regression tests pinning the three filter contracts this refactor has to
preserve: internal ADK events stay excluded whatever else the event carries,
a compaction event is not empty but is still branch-filtered, and a content
with parts but an empty role still counts as empty.
@adk-bot adk-bot added the core [Component] This issue is related to the core interface and implementation label Jul 31, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

core [Component] This issue is related to the core interface and implementation

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants