perf(context): skip O(N) calculateTokenBreakdown when tracer is disabled#26489
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request optimizes the context rendering pipeline by ensuring that resource-intensive diagnostic operations are only performed when tracing is explicitly enabled. By checking the tracer's status before evaluating arguments for log events, the application avoids unnecessary computations on every API turn, leading to improved performance for users with context management enabled. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
The pull request introduces a check for the tracer's enabled state before logging events during the rendering process in packages/core/src/context/graph/render.ts. To support this, a new isEnabled getter was added to the ContextTracer class in packages/core/src/context/tracer.ts. I have no feedback to provide.
JS evaluates function arguments before calling the function, so calculateTokenBreakdown(nodes) was running on every single API turn even though tracer.logEvent immediately returned when tracing is disabled (the common case). Add get isEnabled() to ContextTracer and guard the two expensive tracer.logEvent calls in render.ts: - calculateTokenBreakdown(nodes) — O(N*parts) loop with redundant estimateTokenCountSync calls per part - Object.fromEntries(protectionReasons) — O(N) map copy Both calls now run only when GEMINI_CONTEXT_TRACE_DIR is set.
bb13053 to
acfe282
Compare
Summary
render.tseagerly evaluatedenv.tokenCalculator.calculateTokenBreakdown(nodes)as an argument totracer.logEvent(...)on every API turn, even thoughlogEventimmediately returnsif (!this.enabled). JS evaluates arguments before the function call, so the O(N×parts) computation always ran.get isEnabled(): booleantoContextTracerand guarded the two expensive calls behindif (tracer.isEnabled):calculateTokenBreakdown(nodes)— iterates all nodes, callsestimateTokenCountSyncper part for categorizationObject.fromEntries(protectionReasons)— O(N) map copyGEMINI_CONTEXT_TRACE_DIRis set (tracing opt-in, off by default).Impact: Applies to users with
contextManagement.enabled: true. ThegeneralistProfilebudget is configured, so the budget branch ofrender()fires on every turn for those users.Test plan
GEMINI_CONTEXT_TRACE_DIRis set, trace logs still containEstimation CalibrationandProtection Auditevents