perf(selective): Add lightweight skip path in ChunkedDecoder#855
Open
kewang1024 wants to merge 2 commits into
Open
perf(selective): Add lightweight skip path in ChunkedDecoder#855kewang1024 wants to merge 2 commits into
kewang1024 wants to merge 2 commits into
Conversation
added 2 commits
June 11, 2026 01:20
…incubator#853) Summary: Improve accuracy and consistency of ChunkedDecoder section: fix function labels with `decoder_.`/`streamIndex_->` prefixes, unify box colors, swap panel order, remove redundant callouts, move `encoding_->skip(n)` to its own section, and update `EncodingFactory::create(chunkData)`. Differential Revision: D108181646
Summary: - **Problem**: `skipWithoutIndex()` called `loadNextChunk()` for every chunk, eagerly building the full encoding tree (Nullable → Dictionary → RLE → ...) via `EncodingFactory::create()` just to discard it with `skip(remainingValues_)` - **Fix**: Introduce `skipNextChunk()` — reads only the chunk header + `EncodingPrefix` row count (few bytes), skips whole chunks by pointer advance. Falls back to `EncodingFactory::create()` only for the final partial-skip chunk - **Why here, not lazy in `loadNextChunk()`**: `loadNextChunk()` has 8 other callers (`readWithVisitorImpl`, `decodeNullable`, `testNulls`, etc.) — all immediately read from the encoding, making lazy construction useless. `skipWithoutIndex` was the only caller building expensive trees to throw away - **Perf**: Skip 990K values across 990 chunks: 2450us → 144us (~17x faster) Differential Revision: D106833350
meta-cla
Bot
added
the
CLA Signed
|
@kewang1024 has exported this pull request. If you are a Meta employee, you can view the originating Diff in D106833350. |
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:
skipWithoutIndex()calledloadNextChunk()for every chunk, eagerly building the full encoding tree (Nullable → Dictionary → RLE → ...) viaEncodingFactory::create()just to discard it withskip(remainingValues_)skipNextChunk()— reads only the chunk header +EncodingPrefixrow count (few bytes), skips whole chunks by pointer advance. Falls back toEncodingFactory::create()only for the final partial-skip chunkloadNextChunk():loadNextChunk()has 8 other callers (readWithVisitorImpl,decodeNullable,testNulls, etc.) — all immediately read from the encoding, making lazy construction useless.skipWithoutIndexwas the only caller building expensive trees to throw awayDifferential Revision: D106833350