Add chunked reader fast path#177
Merged
Merged
Conversation
Apply jmh.warmupIterations/iterations/fork/timeOnIteration/warmup property overrides after the bench.profile when block so short-form CLI flags can override profile defaults during ad-hoc gcprof/stackprof runs.
The v2 reader I/O paths routed every char through a coroutine sequence builder (`BufferedReader.toCharSequence` and `Source.toCharSequence`), which allocated a Continuation per character and produced the avgt × thrpt ≈ 3 long-tail divergence flagged in issue #172's primary profile. Add an eager chunked parser that fills a CharArray buffer and walks it directly, using double buffering to carry the next-char lookahead across chunk boundaries. The public lazy `Sequence<List<String>>` API is unchanged; only the I/O wrappers and an internal pipeline helper are rewired. - `ParseStateMachine.reset()` lets `SequenceParser` reuse one machine instance across rows (no per-row alloc of machine / StringBuilder / fields ArrayList). - `parseRowsFromChunks((CharArray) -> Int, dialect, stripBom)` is the new internal entry point; per-char `sequence { yield(c) }` is gone. - `CsvReader.applyPipeline` exposes the skipEmptyLine + field-count policy stages for I/O wrappers to drive directly. - JVM `read(InputStream)` / `readFromFile(File)` wrap `BufferedReader.read(CharArray)`; kotlinx-io `read(Source)` writes decoded code points straight into the chunk buffer.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## version_2_0_0 #177 +/- ##
=================================================
+ Coverage 93.92% 94.87% +0.94%
=================================================
Files 22 22
Lines 461 507 +46
Branches 107 116 +9
=================================================
+ Hits 433 481 +48
+ Misses 16 13 -3
- Partials 12 13 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Add coverage for the new chunked reader fast path so the PR diff is no longer below the codecov threshold: - SequenceParserTest: drive `parseRowsFromChunks` directly with a custom `(CharArray) -> Int` source, including small-buffer chunk boundary swaps, CR/LF on a chunk boundary, the `require(bufferSize >= 2)` guard, BOM strip on/off (default), and an unterminated quote whose tail-flush takes the null-result branch. - CsvReaderJvmIoTest: exercise the I/O pipeline with skipEmptyLine and with an input larger than the default 8 KB chunk so the double buffer swap runs end-to-end. - CsvReaderPathSmokeTest: call the kotlinx-io Path overloads of readFromFile/readAllFromFile with default options, and read a multi-chunk file so the kotlinx-io chunk reader exits via its `index >= limit` branch.
jsoizo
added a commit
that referenced
this pull request
May 22, 2026
Add follow-up coverage for the chunked reader fast path introduced in PR #177: - doubled quote `""` straddling a chunk boundary, so the cross-buffer next-char lookahead has to find the second `"` at nextBuffer[0] for skipCount=1 to do the right thing - explicit-escape `\\<target>` straddling a chunk boundary, same cross-buffer next-char path with a non-self escape char - lone CR at a chunk end followed by a non-LF char in the next chunk, so the CR terminator must not consume the next field char as part of CRLF - supplementary code point (U+1F600) at the parseRowsFromChunks layer where it just passes through as ordinary chars, and at the kotlinx-io Source layer with the 😀 high surrogate at index `buffer.size - 2` so the low surrogate must land on the reserved last slot — a regression in `limit = buffer.size - 1` would overflow Also rewrite the existing chunked-path test comments to lead with why-the-test-exists instead of restating the parser branch.
1 task
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
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
Sequence<Char>reader API intactCharArraybuffer directly with double buffering for next-char lookaheadread(InputStream)/readFromFile(File)and kotlinx-ioread(Source)through the chunked pathParseStateMachine.reset()so the state machine,StringBuilder, andfieldsArrayList are reused across rowsCsvReader.applyPipelineso I/O wrappers can attachskipEmptyLine+ field-count policy without rerouting chars through aSequence<Char>Verification
./gradlew checkbenchmark/parityrow-by-row HARD equivalence test passesBenchmark results
Measured on the same machine as issue #172 (Apple M4 / 32 GB / JDK 21.0.6 / JMH 1.36, seed=42),
primaryprofile (warmup=5 / iter=5 / fork=2 / 10s).7fe79b4). Reader sources between7fe79b4and this branch are touched only by this PR (ParseStateMachine.kt,SequenceParser.kt,CsvReader.kt,ReaderIo.kt,ReaderIoJvm.kt), so the issue numbers are a valid Before for the reader workloads.cdf3284), measured 2026-05-22.Throughput (ops/ms, higher is better)
I/O paths (the workloads that drove the issue's long-tail divergence):
readAll(InputStream)readAll(InputStream)readAll(InputStream)readAll(File)readAll(File)readAll(File)open(file){ asSequence().count() }open(file){ asSequence().count() }open(file){ asSequence().count() }String paths (already v2-favoured; included for completeness):
readAll(String)readAll(String)readAll(String)readAllWithHeader(String)readAllWithHeader(String)readAllWithHeader(String)Average time (ms/op, lower is better)
I/O paths (Before suffered the long-tail outliers —
thrpt × avgt ≈ 3):readAll(InputStream)readAll(InputStream)readAll(InputStream)readAll(File)readAll(File)readAll(File)open(file){ asSequence().count() }open(file){ asSequence().count() }open(file){ asSequence().count() }Notes
thrpt × avgt ≈ 3on Before) is resolved on After. Examples:readAll(InputStream)MEDIUM0.00772 × 129.59 ≈ 1.00,readAll(File)MEDIUM0.00740 × 134.31 ≈ 0.99. Long-tail Continuation allocations from the per-charsequence { yield(c) }are gone.readAll(File)MEDIUM: Before876.29 ± 158.70 ms/op≈ 18% noise → After134.31 ± 2.64≈ 2%), consistent with the parser no longer producing GC-driven outliers.String.asSequence()iterator does not allocate Continuations). The chunked path is a small additional win (+3–8%) but there was no long-tail there to begin with.gc.alloc.rate.norm(B/op) still sits at v1×1.30–1.36 on I/O paths after this PR — this is the structural per-row allocation (e.g.ArrayList.toList()copies,InputStreamReaderinternal char buffer) that does not produce long-tails. Tracked as a follow-up for v2.0.x; not a blocker for the long-tail goal of this PR.alloc/opparity after this PR. Tracked as a follow-up; not addressed here.Reproduction
```bash
./gradlew :benchmark:v2:jmh -Pbench.profile=primary -Pjmh.include='.ReadBenchmarksV2.'
```