From 01b3d68b1c94c734b2ddb1c62d12a74d39e7b859 Mon Sep 17 00:00:00 2001 From: Dixing Xu Date: Fri, 31 Jul 2026 19:49:29 +0000 Subject: [PATCH] perf(flows): remove redundant event scans in prompt assembly _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. --- src/google/adk/flows/llm_flows/contents.py | 96 ++++++++++++--- .../flows/llm_flows/test_contents.py | 115 ++++++++++++++++++ 2 files changed, 191 insertions(+), 20 deletions(-) diff --git a/src/google/adk/flows/llm_flows/contents.py b/src/google/adk/flows/llm_flows/contents.py index 9b95a3a813..399e4c70c0 100644 --- a/src/google/adk/flows/llm_flows/contents.py +++ b/src/google/adk/flows/llm_flows/contents.py @@ -146,9 +146,14 @@ def _rearrange_events_for_async_function_responses_in_history( events: list[Event], ) -> list[Event]: """Rearrange the async function_response events in the history.""" + # `get_function_responses()` and `get_function_calls()` each rebuild a list + # by rescanning every part, and upstream calls them repeatedly per event. + # Compute both once per event and reuse them below. + responses_per_event = [event.get_function_responses() for event in events] + calls_per_event = [event.get_function_calls() for event in events] + function_call_id_to_response_events_index: dict[str, int] = {} - for i, event in enumerate(events): - function_responses = event.get_function_responses() + for i, function_responses in enumerate(responses_per_event): if function_responses: for function_response in function_responses: function_call_id = function_response.id @@ -158,14 +163,14 @@ def _rearrange_events_for_async_function_responses_in_history( return events result_events: list[Event] = [] - for event in events: - if event.get_function_responses(): + for i, event in enumerate(events): + if responses_per_event[i]: # function_response should be handled together with function_call below. continue - elif event.get_function_calls(): + elif calls_per_event[i]: function_response_events_indices = set() - for function_call in event.get_function_calls(): + for function_call in calls_per_event[i]: function_call_id = function_call.id if function_call_id in function_call_id_to_response_events_index: function_response_events_indices.add( @@ -287,6 +292,13 @@ def _rearrange_events_for_latest_function_response( return result_events +_INTERNAL_FUNCTION_NAMES = frozenset({ + 'adk_framework', + REQUEST_EUC_FUNCTION_CALL_NAME, + REQUEST_CONFIRMATION_FUNCTION_CALL_NAME, +}) + + def _is_part_invisible( p: types.Part, *, include_thoughts: bool = False ) -> bool: @@ -439,12 +451,51 @@ def _should_include_event_in_context( ev_iso = getattr(event, 'isolation_scope', None) if ev_iso != isolation_scope: return False + + # Single pass over the parts. `_contains_empty_content` and the three + # internal-event predicates below each re-walk `event.content.parts`; this + # derives the same two facts once. The returned expression keeps upstream's + # operand order and short-circuit structure exactly. + content = event.content + all_parts_invisible = True + is_internal_event = False + if content and content.parts: + for part in content.parts: + function_call = part.function_call + if function_call is not None: + if function_call.name in _INTERNAL_FUNCTION_NAMES: + is_internal_event = True + break + all_parts_invisible = False + continue + function_response = part.function_response + if function_response is not None: + if function_response.name in _INTERNAL_FUNCTION_NAMES: + is_internal_event = True + break + all_parts_invisible = False + continue + if all_parts_invisible and not _is_part_invisible( + part, include_thoughts=include_thoughts + ): + all_parts_invisible = False + + if is_internal_event: + return False + + if event.actions and event.actions.compaction: + contains_empty_content = False + else: + contains_empty_content = ( + not content + or not content.role + or not content.parts + or all_parts_invisible + ) and (not event.output_transcription and not event.input_transcription) + return not ( - _contains_empty_content(event, include_thoughts=include_thoughts) + contains_empty_content or not _is_event_belongs_to_branch(current_branch, event) - or _is_adk_framework_event(event) - or _is_auth_event(event) - or _is_request_confirmation_event(event) ) @@ -671,16 +722,19 @@ def _copy_content_for_request( if not parts: return new_content + if not strip_client_function_call_ids: + new_content.parts = [part.model_copy() for part in parts] + return new_content + new_parts = [] for part in parts: new_part = part.model_copy() - if strip_client_function_call_ids: - fc = new_part.function_call - if fc and fc.id and fc.id.startswith(AF_FUNCTION_CALL_ID_PREFIX): - new_part.function_call = fc.model_copy(update={'id': None}) - fr = new_part.function_response - if fr and fr.id and fr.id.startswith(AF_FUNCTION_CALL_ID_PREFIX): - new_part.function_response = fr.model_copy(update={'id': None}) + fc = new_part.function_call + if fc and fc.id and fc.id.startswith(AF_FUNCTION_CALL_ID_PREFIX): + new_part.function_call = fc.model_copy(update={'id': None}) + fr = new_part.function_response + if fr and fr.id and fr.id.startswith(AF_FUNCTION_CALL_ID_PREFIX): + new_part.function_response = fr.model_copy(update={'id': None}) new_parts.append(new_part) new_content.parts = new_parts return new_content @@ -741,9 +795,11 @@ def _get_contents( ) ] - has_compaction_events = any( - e.actions and e.actions.compaction for e in raw_filtered_events - ) + has_compaction_events = False + for e in raw_filtered_events: + if e.actions and e.actions.compaction: + has_compaction_events = True + break if has_compaction_events: events_to_process = _process_compaction_events(raw_filtered_events) diff --git a/tests/unittests/flows/llm_flows/test_contents.py b/tests/unittests/flows/llm_flows/test_contents.py index 7243fbe7f0..e95ffdc3fa 100644 --- a/tests/unittests/flows/llm_flows/test_contents.py +++ b/tests/unittests/flows/llm_flows/test_contents.py @@ -2041,3 +2041,118 @@ def test_task_input_user_content_preserves_non_ascii(): assert "שלום עולם" in text assert "北京" in text assert "\\u" not in text + + +def _internal_call_event(name: str, **kwargs) -> Event: + """An event whose only part is an internal ADK function call.""" + return Event( + invocation_id="inv_internal", + author="agent", + content=types.Content( + role="model", + parts=[ + types.Part( + function_call=types.FunctionCall( + id="fc-1", name=name, args={} + ) + ) + ], + ), + **kwargs, + ) + + +@pytest.mark.parametrize( + "internal_name", + [ + "adk_framework", + REQUEST_EUC_FUNCTION_CALL_NAME, + REQUEST_CONFIRMATION_FUNCTION_CALL_NAME, + ], +) +def test_internal_events_excluded_even_with_transcription(internal_name): + """Internal ADK events stay excluded whatever else the event carries. + + The exclusion is independent of the emptiness check, so an internal event is + filtered out even when a transcription or a compaction action would otherwise + make it non-empty. + """ + compaction = EventCompaction( + start_timestamp=1.0, + end_timestamp=2.0, + compacted_content=types.Content( + role="model", parts=[types.Part(text="summary")] + ), + ) + + plain = _internal_call_event(internal_name) + with_transcription = _internal_call_event( + internal_name, + input_transcription=types.Transcription(text="hello"), + ) + with_compaction = _internal_call_event( + internal_name, actions=EventActions(compaction=compaction) + ) + + for event in (plain, with_transcription, with_compaction): + assert not contents._should_include_event_in_context(None, event) + + +def test_internal_event_with_empty_role_and_transcription_excluded(): + """The internal-event exclusion does not depend on the content role. + + A transcription keeps the event from being empty, and an empty role keeps the + parts from counting as visible content; the internal function call must still + exclude the event. + """ + event = Event( + invocation_id="inv_internal_empty_role", + author="agent", + content=types.Content( + role="", + parts=[ + types.Part( + function_call=types.FunctionCall( + id="fc-1", name="adk_framework", args={} + ) + ) + ], + ), + input_transcription=types.Transcription(text="hello"), + ) + + assert not contents._should_include_event_in_context(None, event) + + +def test_compaction_event_included_but_still_branch_filtered(): + """A compaction event is not empty, but the other filters still apply.""" + compaction = EventCompaction( + start_timestamp=1.0, + end_timestamp=2.0, + compacted_content=types.Content( + role="model", parts=[types.Part(text="summary")] + ), + ) + event = Event( + invocation_id="inv_compaction", + author="agent", + branch="root.other_agent", + content=types.Content(role="model", parts=[types.Part(text="hi")]), + actions=EventActions(compaction=compaction), + ) + + assert contents._should_include_event_in_context(None, event) + assert not contents._should_include_event_in_context( + "root.third_agent", event + ) + + +def test_content_with_parts_but_empty_role_is_empty(): + """Emptiness depends on the role as well as on the parts.""" + event = Event( + invocation_id="inv_empty_role", + author="agent", + content=types.Content(role="", parts=[types.Part(text="hi")]), + ) + + assert not contents._should_include_event_in_context(None, event)