Skip predicate evaluation while seeking parquet iterators to a target row#7635
Skip predicate evaluation while seeking parquet iterators to a target row#7635mapno wants to merge 5 commits into
Conversation
… row SyncIterator.SeekTo scanned toward the seek target by calling next(), which evaluated the column predicate on every value and returned them one call at a time, only for SeekTo to discard everything before the target. The predicate's verdict on those values is irrelevant. The seek scan now compares row numbers first and only evaluates the predicate for values at/after the target, consuming the read buffer inline instead of one next() call per value. TraceQL benchmarks on a real vparquet5 block: geomean -5.5% (up to -9.2%) on search, -18.2% on a filtered metrics query. Unfiltered scans and plain Next() iteration unchanged.
There was a problem hiding this comment.
Pull request overview
This PR optimizes SyncIterator.SeekTo in pkg/parquetquery by avoiding per-value predicate evaluation for values that are scanned and discarded while seeking to a target row, improving performance for join-style TraceQL queries while keeping result semantics unchanged.
Changes:
- Updates
SyncIterator.SeekTo/nextto skip predicate evaluation for values strictly before the seek target, while still applying the predicate at/after the target. - Refactors iterator buffer refill/page advancement logic into a dedicated
fill()helper. - Adds targeted tests for SeekTo+predicate interaction and a benchmark modeling repeated forward seeks.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| pkg/parquetquery/iters.go | Implements seek-aware scanning that bypasses predicate checks for pre-target values and factors buffer refill logic into fill(). |
| pkg/parquetquery/iters_test.go | Adds tests asserting predicate behavior around SeekTo and introduces a benchmark for forward-seek access patterns. |
| .chloggen/synciterator-seek-skip-filter.yaml | Adds a changelog entry documenting the iterator seek performance enhancement. |
| if seeking && c.beforeSeekTarget() { | ||
| continue | ||
| } | ||
|
|
||
| if c.filter != nil && !c.filter.KeepValue(*v) { | ||
| continue | ||
| } |
There was a problem hiding this comment.
I've already tried this variant while developing the PR: flipping seeking mid-loop converts it from a loop-invariant (free, perfectly-predicted branch) into loop-carried state, which grows the frame and regressed seek microbenchmarks by ~14%
|
|
||
| // Seek scratch state owned by next(). Copied from its arguments so the | ||
| // hot loop can compare in place without growing next's stack frame. | ||
| seekTo RowNumber |
There was a problem hiding this comment.
Can this be simplified by passing them into the BeforeSeekTo function? Instead of member vars?
What this PR does:
Skips predicate evaluation for values scanned past during
SyncIterator.SeekTo.SeekToscans toward the seek target by callingnext(), which evaluates the column predicate on every value — only forSeekToto discard everything before the target.When seeking,
nextskips values before the target without evaluating the filter, then filters normally from the target onward — one loop, no separate seek path. The target is kept as iterator scratch state and compared in place by an inlineable helper, keeping 32-byteRowNumbercopies out of the hot loop. Results are identical.{span.http.host != `` && span.http.flavor=2} | rate() by (...)): −18.3%Next()iteration and unfiltered scans: unchangedbenchstat: vParquet5 block benchmarks (main vs PR, n=10)
benchstat: BenchmarkSyncIteratorSeekTo (main vs PR, n=8)
Which issue(s) this PR fixes:
N/A
Checklist
.chloggen/(runmake chlog-new, ormake chlog-new FILENAME=<name>to override the default branch-name file; see.chloggen/README.md)