lucasfang opened a new pull request, #366:
URL: https://github.com/apache/paimon-cpp/pull/366
<!-- PR title: perf(read): warm up only raw bytes by default -->
### Purpose
Linked issue: close #365
`WarmupLevel::DECODED` was the default, so every prefetching read started a
background decode loop on the next file while the current one was still being
consumed, holding the decoded batches of two files at once per reader in flight
— multiplied by the number of sorted runs in a merge-on-read section, since
`LoserTree` warms one file per leaf. That memory is committed before the query
reaches the file and is wasted when the scan stops early, while most of the
latency it hides is the remote fetch that the cheaper `RAW` level already
covers.
This changes the default to `WarmupLevel::RAW` and leaves `DECODED` as an
explicit opt-in. No enum value, signature, or plumbing change; every level
keeps its behavior, and a caller that sets a level explicitly is unaffected.
- `ReadContextBuilder` defaults `warmup_level_` to `RAW` in both the member
initializer and `Reset()`, so a builder reused after `Finish()` gets the same
default.
- `DataFileReadOptions::warmup_level` defaults to `RAW`, which is what an
internal caller that fills read options directly gets.
- The `WarmupLevel` enum and `ReadContextBuilder::SetWarmupLevel()` docs
move the "this is the default" note to `RAW`.
One consequence to note: `RAW` warms through the read-ahead cache and falls
back to `NONE` when that cache is off, so a reader built with
`SetReadAheadCacheEnabled(false)` now warms nothing by default, where it
previously got a background decode. Such a reader has to ask for `DECODED`
explicitly.
### Tests
No new case: the level-specific behavior is unchanged, and the existing
coverage iterates all three levels rather than relying on which one is the
default — `PrefetchFileBatchReaderImplTest.TestWarmupLevelNone` /
`.TestWarmupLevelRaw` / `.TestWarmupLevelDecoded` (each over `{parquet, orc}` x
`{read-ahead cache on, off}`),
`DataFileReaderFactoryTest.OpenForwardsWarmupLevelToPrefetchReader`, and
`SystemTableTest.TestNewReadPropagatesWarmupLevel`.
Updated the cases that pin the default: `ReadContextTest.TestDefaultValue`
and `ReadContextTest.TestSetWarmupLevel` (the post-`Finish()` reset now expects
`RAW`). `ReadContextTest.TestSetContent` set `RAW`, which is now the default
and would no longer catch a dropped setter, so it sets `DECODED` instead.
Ran: `paimon-common-test` (1654 cases, 1652 passed, 2 pre-existing skips)
and `paimon-core-test` (2187 cases, all passed), plus the directed
`ReadContextTest.*:SystemTableTest.*` and `DataFileReaderFactoryTest.*`
filters. `pre-commit run --files <changed files>` and `git diff --check` are
clean. Integration tests and benchmarks were not run.
### API and Format
A default-value change to public API behavior in `include/paimon/`:
`ReadContextBuilder::SetWarmupLevel()` now defaults to `WarmupLevel::RAW`. No
signature, ABI, storage format, or protocol change.
### Documentation
Not a new feature. The doc comments on `WarmupLevel` and `SetWarmupLevel()`
are updated in place, which is what `docs/source/api/read.rst` renders.
### Generative AI tooling
Generated-by: Qoder
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
