Skip to content

perf(loki.process): Reduce pipeline latency with buffered inter-stage channels - #6875

Draft
csmarchbanks wants to merge 1 commit into
loki-process-benchmarksfrom
loki-process-buffered-channels
Draft

perf(loki.process): Reduce pipeline latency with buffered inter-stage channels#6875
csmarchbanks wants to merge 1 commit into
loki-process-benchmarksfrom
loki-process-buffered-channels

Conversation

@csmarchbanks

@csmarchbanks csmarchbanks commented Aug 12, 2026

Copy link
Copy Markdown

Brief description of Pull Request

Alternative, smaller-diff approach to the same problem as #6873: loki.process gives every pipeline stage its own unbuffered channel, forcing a synchronous rendezvous (sender blocks until the receiver is scheduled) for every entry at every stage boundary. This PR keeps the existing goroutine-per-stage architecture entirely as-is and only changes channel construction: every inter-stage channel now goes through one helper, newEntryChan() (pipeline.go), with a buffer of 16 instead of 0. No stage logic, interfaces, or goroutine topology change.

This PR is now chained on top of #6879, which holds the shared benchmark commit as a common base (that commit also picked up a benchmark-harness fix: the drain goroutine wasn't awaited, so a sub-benchmark's leftover work could bleed CPU into the next one's timing). This branch itself contains only the one-constant change (b79220f83). git checkout loki-process-benchmarks vs git checkout loki-process-buffered-channels reproduces the numbers below.

benchstat, GOMAXPROCS=2, -count=10, re-run against the fixed benchmark harness (this supersedes the numbers previously posted here, which predated that fix):

                                                             │       base (#6879)        │        this branch (b79220f83)       │
                                                             │           sec/op          │    sec/op     vs base               │
PipelineManyRules/rules=1-2                                              708.3n ±  2%   549.8n ± 2%  -22.37% (p=0.000 n=10)
PipelineManyRules/rules=10-2                                            2299.5n ±  2%   628.2n ± 1%  -72.68% (p=0.000 n=10)
PipelineManyRules/rules=50-2                                             9.973µ ±  1%   1.874µ ± 1%  -81.21% (p=0.000 n=10)
PipelineManyRules/rules=100-2                                           19.051µ ±  1%   3.384µ ± 1%  -82.24% (p=0.000 n=10)
PipelineManyRules/rules=500-2                                            84.05µ ±  1%   13.63µ ± 3%  -83.79% (p=0.000 n=10)
PipelineManyRules/rules=1000-2                                          152.10µ ±  1%   24.43µ ± 4%  -83.94% (p=0.000 n=10)
PipelineOneMultilineAmongManyRules/all_sync-2                           149.30µ ±  1%   24.15µ ± 4%  -83.83% (p=0.000 n=10)
PipelineOneMultilineAmongManyRules/one_multiline_at_start-2             151.46µ ±  2%   24.65µ ± 3%  -83.73% (p=0.000 n=10)
PipelineOneMultilineAmongManyRules/one_multiline_in_middle-2            151.25µ ±  2%   24.47µ ± 3%  -83.82% (p=0.000 n=10)
PipelineOneMultilineAmongManyRules/one_multiline_at_end-2               148.66µ ±  2%   24.36µ ± 4%  -83.62% (p=0.000 n=10)
PipelineManyStreamsSaturated-2                                          173.68m ±  2%   71.66m ± 6%  -58.74% (p=0.000 n=10)
geomean                                                                  73.72µ         16.08µ       -78.19%

                               │       base (#6879)        │        this branch (b79220f83)        │
                               │        entries/sec        │  entries/sec    vs base                │
PipelineManyStreamsSaturated-2              9.212k ±  2%   22.332k ± 5%  +142.42% (p=0.000 n=10)

                               │       base (#6879)        │        this branch (b79220f83)        │
                               │        B/op / allocs/op   │      B/op / allocs/op   vs base        │
PipelineManyStreamsSaturated-2       6.612Mi / 88.37k     69.871Mi / 127.6k   +956.70% / +44.38% (p=0.000 n=10)

The single-stream latency win here is larger across the board than #6873's full redesign (e.g. -83.9% vs -77.1% at rules=1000), consistent with the original sweep. The concurrent-throughput win is real but smaller than the full redesign (+142% vs +577% entries/sec) — and this re-run also surfaces a real cost the earlier numbers didn't show: PipelineManyStreamsSaturated memory jumps +957% (6.6MiB → 69.9MiB per op) and allocations +44%, versus #6873's redesign which cuts both (-84% / -91%). Buffering trades memory for latency by letting more entries sit in flight at once; the full redesign avoids that trade-off entirely by removing the per-stage goroutines and channels rather than padding them.

Buffer size was swept (0/1/2/4/8/16/32/64) at several GOMAXPROCS levels before settling on 16: single-stream latency keeps improving monotonically at least to buffer=32 with no core-count-dependent downside found, but the concurrent-streams throughput benchmark has a real peak (varies by ~4-16 depending on GOMAXPROCS) and loses most of its gain by buffer=64. 16 balances both. (This sweep itself predates the harness fix and hasn't been re-run; the memory trade-off above means it's worth re-sweeping with B/op as a second axis, not just throughput.)

Verified with go test -race across internal/component/loki/process/... and golangci-lint (no new findings).

Pull Request Details

Issue(s) fixed by this Pull Request

Notes to the Reviewer

PR Checklist

  • Documentation added
  • Tests updated
  • Config converters updated
  • This pull request was substantially generated with AI assistance (see the GenAI policy)

// (instead of requiring a synchronous unbuffered rendezvous) reduce the
// goroutine park/wake cost that dominates a pipeline with many stages? 0
// keeps the original unbuffered behavior.
const entryChanBufferSize = 16

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the benchmarks across a variety of buffer sizes and with trying to set to ~GOMAXPROCS. 16 seemed like a reasonable balance of improving performance without too much complexity or memory across all core counts.

Every stage-to-stage handoff used an unbuffered channel, which forces a
synchronous rendezvous (sender blocks until a receiver is ready) for every
single entry at every single stage boundary. That rendezvous is where the
goroutine park/wake cost identified in BenchmarkPipelineManyRules and
BenchmarkPipelineManyStreamsSaturated actually comes from.

Giving each channel created via the new newEntryChan a small buffer lets a
send complete without waiting for the receiving goroutine to actually be
scheduled, decoupling adjacent stages' scheduling instead of forcing them
to synchronize on every entry. Swept buffer sizes 0/1/4/16/64 and GOMAXPROCS
2/4/8/18 (see conversation): the improvement is consistent across every
GOMAXPROCS level tried, unlike the larger fuse-into-one-goroutine redesign,
which trades away single-idle-stream latency for a multi-stream throughput
win. 16 is the sweet spot found in this sweep; benefit drops off by 64.

This is deliberately a minimal, surgical change: only channel construction
sites change (all now go through newEntryChan), nothing about the stage
interfaces, goroutine topology, or match/pipeline structure is touched.

Assisted-by: Claude Sonnet 5
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant