Cache type service readers in page readers - #955
Conversation
|
|
||
| // Reuse type-specific readers and the deletion predicate across repeated page reads. | ||
| private PageDataValueReader batchDataValueReader; | ||
| private PageDataBlockValueReader blockValueReader; |
There was a problem hiding this comment.
Cache these type-specific readers for the lifetime of the page reader. The data type is immutable, so repeated page scans can reuse the dispatch result and avoid rebuilding service lambdas.
| long timestamp = timeDecoder.readLong(timeBuffer); | ||
| valueReader.read( | ||
| valueDecoder, valueBuffer, recordFilter, pageData, timestamp, allSatisfy, isDeleted); | ||
| batchDataValueReader.read( |
There was a problem hiding this comment.
Keep the deletion predicate bound once per reader so every decoded row uses the same instance without allocating a method reference for each page read.
| private final Decoder valueDecoder; | ||
|
|
||
| // Reuse the type-specific reader across per-row nextValue calls. | ||
| private PageDataTsPrimitiveValueReader valueReader; |
There was a problem hiding this comment.
Reuse the primitive, batch, and column-builder readers across the value-page APIs. These readers are stateless with respect to the page and only depend on the immutable data type, so lazy initialization preserves construction behavior while removing repeated type dispatch.
| TypeServices.READ_PAGE_VALUE_TO_TSPRIMITIVETYPE_SERVICE.call( | ||
| Type.fromTsDataType(dataType)); | ||
| } | ||
| return valueReader.read(valueDecoder, valueBuffer, timestamp, deletePredicate); |
There was a problem hiding this comment.
Use the cached primitive reader and deletion predicate in the hot per-row path. This removes repeated type dispatch and method-reference creation while preserving decoder ordering and deletion semantics.
| long timestamp = timeDecoder.readLong(timeBuffer); | ||
| PageDataReadStatus status = | ||
| valueReader.read( | ||
| blockValueReader.read( |
There was a problem hiding this comment.
Reuse the cached TsBlock reader for repeated block page reads. Lazy initialization keeps the existing construction path and removes repeated type service lookup from the row loop.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #955 +/- ##
===========================================
- Coverage 64.03% 63.94% -0.10%
===========================================
Files 757 758 +1
Lines 52993 53249 +256
Branches 8342 8446 +104
===========================================
+ Hits 33936 34051 +115
- Misses 17421 17535 +114
- Partials 1636 1663 +27 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2 participants
Summary
Cache type-specific page value readers and the bound deletion predicate in
PageReaderandValuePageReader.Repeated reads reuse the type dispatch result, and each page reader binds its deletion predicate once. Service readers are initialized lazily when the corresponding API is first used; the predicate is initialized during construction.
Performance
Local JMH 1.37 measurements on Windows 11, Intel Core i9-12900, JBR/OpenJDK 21.0.8. JVM arguments:
-Xms512m -Xmx512m -XX:+UseG1GC -XX:ActiveProcessorCount=2. Each scenario uses one thread and five independent JVM runs, with four 500 ms warmup iterations and six 500 ms measurement iterations per run, using-prof gc. Values below are medians of the five per-run means. Lower ns/op and B/op are better; time reduction is(baseline - cached) / baseline.Measurement scope: these results were collected for the earlier
ValuePageReadersnapshot with cachedvalueReaderanddeletePredicate, before adding the batch/column-builder caches and thePageReaderchanges in commit4fabada0. They characterize thenextValueoptimization, not the exact final PR revision or its other APIs. The measured snapshot is identified by source SHA-256b0694ce4ef3e75a4bf1a3a0d5ad6e9c0f7a5ed43441b35b78c85337eb4cc35b0.scanreuses four PLAIN-encoded pages of 1,024 rows, callingnextValuein row/column order and consuming results with a JMH Blackhole. Its 4,096 calls include bitmap-null and deleted rows; buffer rewinds are included and amortized across those calls.MIXEDuses INT32, INT64, DOUBLE, and BOOLEAN columns.MIXED_NULL_DELETEadditionally has 25% bitmap-null rows per column and a deletion interval covering the last quarter of timestamps. This measures in-memory decoding, without file I/O or decompression.Compared with the implementation before caching
Baseline:
e00a1e43969efb9f3a9725fbd8e6d75595fd7e5d, after the type-service refactor. These variants use the same dependency classes.Reader-only allocation matches the baseline in every scenario. Caching the predicate additionally reduces allocation in the mixed scenarios. The single-type measurements do not show a consistent speedup.
Compared with the version before the type-service refactor
Baseline:
9be35b9bea2c95dd6f0201aeb8f04255775f2a9f, the parent of02b503b92416a04fcfb360f446c08ef3e0978ed2. Its completejava/commonandjava/tsfilemodules were built and used with the same harness and JMH settings.The pre-refactor runs were collected later, rather than interleaved with the cached variants, and include changes in dependency classes. These percentages are descriptive comparisons, not an isolated causal estimate of the refactor. Run-to-run variability is substantial: MIXED ranges from 5.639–8.969 ns/op before the refactor and 6.674–7.976 ns/op with caching. The overlapping ranges do not establish a consistent 8.4% regression.
New-page first value
This separate INT64 benchmark includes buffer wrapping, decoder/reader construction, bitmap parsing, and the first
nextValuecall. It measures a new reader in a warmed JVM, not cold JVM startup.The timing ranges overlap, while allocation increases by 24 B per new reader in the measured cached variant. All four variants passed value-by-value checks for all five scenarios, two complete page replays, and the first value of a new page. These microbenchmarks do not establish end-to-end query throughput gains.
Validation
mvn.cmd -P with-java -pl java/tsfile -am test '-Dtest=PageReaderTest,TsFileLastReaderTest' '-Dsurefire.failIfNoSpecifiedTests=false'git diff --checkpassed