perf: Reduce allocations on the response-body streaming path - #2252
Open
albertoperdomo2 wants to merge 1 commit into
Open
perf: Reduce allocations on the response-body streaming path#2252albertoperdomo2 wants to merge 1 commit into
albertoperdomo2 wants to merge 1 commit into
Conversation
Signed-off-by: Alberto Perdomo <aperdomo@redhat.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What type of PR is this?
/kind bug
What this PR does / why we need it:
The EPP response-body hot path allocates on every streaming chunk for operations that are either unnecessary at that phase or can be simply avoided. Under sustained load (~40 req/s, streaming,
gpt-oss-120b) GC dominates the CPU profile (lfstack.pop,gcDrainN, andgetemptytogether consume over 50% of total CPU time).Which issue(s) this PR fixes:
This PR targets the three cheapest wins on that path:
Per-phase metadata extraction instead of every-request:
ExtractMetadataValueswas called unconditionally before the phase switch, allocating a map on every message including intermediate response-body chunks that never use metadata. Moved the call into the three cases that consume it (request headers, end-of-stream body, response headers) so intermediate chunks pay nothing.Guarded TRACE logging with
.Enabled()checks:HandleResponseBodyandrunResponseBodyPluginsare called per chunk. Unconditionallogger.V(TRACE).Info(...)calls allocate zap encoder clones even when TRACE is disabled. Added.Enabled()guards on the per-chunk path.Cached
plugin.TypedName()into a local variable: Eachrun*Pluginsloop calledplugin.TypedName()2-4 times per plugin per request (for logging and metrics).TypedName()is an interface method returning a two-string struct; caching it in a local avoids repeated virtual dispatch and copies.Test plan: All existing a new added tests pass.
Profiles (30s window, ~1,240 requests, openai/gpt-oss-120b) collected on the same node under the same load (~40 req/s steady-state). PRE uses the upstream v0.9.0 image; POST uses the
perf/reduce-response-streaming-allocsbuild.Runtime metrics (go_memstats, 30s delta):
perf/reduce-response-streaming-allocs)Response-body path allocations (pprof alloc_objects, focused on HandleResponseBody + related):
perf/reduce-response-streaming-allocs)ExtractMetadataValues(flat)Director.HandleResponseBody(cum)HandleResponseBodyfull path (cum)zapLogger.WithValues(flat)CPU breakdown: GC + memory management (
lfstack.pop,gcDrainN,getempty,gcDrain):perf/reduce-response-streaming-allocs)The top-line allocation reduction is modest (-1.6% bytes) because the optimized functions are small allocators relative to the total, but the GC impact is outsized: the response-body path fires per chunk (high frequency), so small per-call savings compound across thousands of chunks per window. The net effect is a 54% reduction in GC CPU and 18.6% fewer GC cycles under comparable load.
Release note (write
NONEif no user-facing change):NONE