logstore: cut allocation churn on the incremental log hot path#6803
Open
alleneubank wants to merge 2 commits into
Open
logstore: cut allocation churn on the incremental log hot path#6803alleneubank wants to merge 2 commits into
alleneubank wants to merge 2 commits into
Conversation
Every subscriber notification through ContinuingLines cloned the entire span map (one span per pod/build) even when there was nothing to print, and every incremental websocket update through ToLogList re-sent every span. After the PodMonitor fix this was the dominant remaining source of steady-state GC churn under a log storm (200-span session shape). - ContinuingLinesWithOptions returns early when the checkpoint window is empty: 13.3us/203 allocs -> 1.6ns/0 allocs per no-op notification. - Temp stores clone only the spans referenced by the checkpoint window instead of the whole span map (5-line window: 17.5us/247 allocs -> 2.8us/45 allocs); the now-unused cloneSpanMap is deleted. - Incremental ToLogList updates carry only spans referenced by the sent segments: 7.9us/216 allocs -> 430ns/15 allocs. Safe for both clients: `tilt logs` resolves segments strictly against the update that carried them and the web UI merges spans additively; locked in by TestLogIncrementalSpansScopedToSegments. - segmentsFromBytes preallocates segments via newline count (100-line append: 10 -> 3 allocs). Benchmarks live in the new logstore_bench_test.go. Full-path k8s harness verdict: pass, all floors green, log ingestion at 100.9% of baseline.
Rendering a log line cost ~8 heap allocations: a fresh logLineBuilder
and segments slice per line, a per-line []LogLine{} returned by build,
two allocations in SourcePrefix (strings.Repeat + fmt.Sprintf), unsized
strings.Builder growth, and append-doubling of the result slice. After
the span-cloning fixes this was the dominant remaining allocation
source in the terminal-stream path (toLogLines at 8.8% of cumulative
allocations under a steady-state log storm).
- One logLineBuilder value is reused across all lines of a toLogLines
call; start/active/reset recycle its segment buffer. newLogLineBuilder
and build are deleted.
- appendTo appends built lines into the caller's result slice instead
of allocating a per-line slice.
- appendSourcePrefix writes the padded manifest prefix directly into
the line's builder with zero allocations. SourcePrefix output is
unchanged, locked by a new characterization test.
- toLogLines pre-counts line-starting segments to size the result
once; buildMainLine/buildSpaceLine/linesToString pre-size their
builders so each line's text is a single allocation.
- ContinuingStringWithOptions deduplicated into linesToString.
Benchmarks (200-span shape, new BenchmarkStringFullStore baselined
before optimizing): full-store render 950us/16039 allocs/2.09MB ->
305us/2003 allocs/513KB; ContinuingLines with 5 new lines 3.1us/45
allocs -> 760ns/8 allocs. Full-path k8s harness verdict: pass, all
floors green, log rate at 93.6% of baseline (floor 80%); toLogLines
cumulative allocations 138.9MB -> 50.9MB in the steady-state profile.
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
1 participant
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.
Summary
After reducing subscriber work on log-only notifications, logstore was still
a major source of steady-state GC under a multi-span log storm:
ToLogListre-sent every spantoLogLinesallocated heavily per rendered lineThis PR:
ContinuingLinesWithOptionswhen the checkpoint windowis empty.
cloneSpanMapfor that path).ToLogListspan payloads to segments actually sent(safe for
tilt logsand the web client; covered byTestLogIncrementalSpansScopedToSegments).and cuts per-line prefix/builder churn in
toLogLines/ prefix helpers.Motivation
On a ~200-span session shape after the PodMonitor skip, span cloning and
line materialization dominated remaining engine alloc profiles during
steady log ingress.
Evidence
logstore_bench_test.go(empty window, narrow window,full-store render, continuing lines).
full-store render ≈ 950µs / 16k allocs → ≈ 305µs / 2k allocs.
band while validating the firehose path.
Test plan
go test ./pkg/model/logstore/go test -bench=. ./pkg/model/logstore/if reviewing numbers