feat(duckdb): overlap IO across two splits per scan thread - #9610
Draft
joseph-isaacs wants to merge 2 commits into
Draft
feat(duckdb): overlap IO across two splits per scan thread#9610joseph-isaacs wants to merge 2 commits into
joseph-isaacs wants to merge 2 commits into
Conversation
reader_try_initialize_scan now takes up to two splits from the file at once, and reader_scan drives them through a buffered (when file order matters for file_row_number) or buffer_unordered stream, so IO for the second split overlaps decode/export of the first. Signed-off-by: "Joe Isaacs" <joe.isaacs@live.co.uk> Signed-off-by: Joe Isaacs <joe.isaacs@live.co.uk>
joseph-isaacs
added
the
action/bench-sql
Claude
github-actions
Bot
removed
the
action/bench-sql
joseph-isaacs
added
the
changelog/performance
Claude
Merging this PR will regress 1 benchmark
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | WallTime | mul_u32_nonnull_avx512 |
5.6 µs | 6.3 µs | -11.05% |
| ⚡ | WallTime | arrow_checked_add_u32_neon[16384] |
20.4 µs | 12.4 µs | +64.48% |
| ⚡ | WallTime | words_gather_scalar_avx2[65536] |
9.4 µs | 8.2 µs | +14.2% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing claude/duckdb-parallel-split-scan-27x5wk (25eaa98) with develop (4f45f72)
Footnotes
-
106 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩
Contributor
Polar Signals Profiling ResultsLatest Run
Previous Runs (8)
Powered by Polar Signals Cloud |
Contributor
Benchmarks: TPC-H SF=1 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.997x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.998x ➖, 0↑ 1↓)
duckdb / vortex-file-compressed / ns (0.997x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.001x ➖, 0↑ 0↓)
File Size Changes (10 files changed, -45.5% overall, 0↑ 10↓)
Totals:
|
Contributor
Benchmarks: FineWeb NVMe 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.020x ➖, 0↑ 1↓)
datafusion / parquet / ns (0.998x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.012x ➖, 0↑ 1↓)
duckdb / parquet / ns (0.988x ➖, 0↑ 0↓)
File Size Changes (2 files changed, -46.4% overall, 0↑ 2↓)
Totals:
|
Contributor
Benchmarks: TPC-H SF=10 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.005x ➖, 0↑ 0↓)
datafusion / parquet / ns (0.998x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.956x ➖, 2↑ 0↓)
duckdb / parquet / ns (0.994x ➖, 1↑ 1↓)
File Size Changes (10 files changed, -45.6% overall, 0↑ 10↓)
Totals:
|
Contributor
Benchmarks: Clickbench on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (1.007x ➖, 0↑ 0↓)
datafusion / parquet / ns (1.001x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.020x ➖, 0↑ 2↓)
duckdb / parquet / ns (1.006x ➖, 1↑ 1↓)
File Size Changes (101 files changed, -39.5% overall, 0↑ 101↓)
Totals:
|
Contributor
Benchmarks: TPC-H SF=1 on S3 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.926x ➖, 2↑ 0↓)
datafusion / parquet / ns (1.039x ➖, 0↑ 1↓)
duckdb / vortex-file-compressed / ns (0.969x ➖, 1↑ 1↓)
duckdb / parquet / ns (0.988x ➖, 0↑ 0↓)
|
Contributor
Benchmarks: FineWeb S3 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.964x ➖, 1↑ 2↓)
datafusion / parquet / ns (1.036x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (1.005x ➖, 0↑ 0↓)
duckdb / parquet / ns (1.539x ❌, 0↑ 7↓)
|
Contributor
Benchmarks: Clickbench Sorted on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.973x ➖, 1↑ 0↓)
datafusion / parquet / ns (1.010x ➖, 0↑ 0↓)
duckdb / vortex-file-compressed / ns (0.994x ➖, 0↑ 0↓)
duckdb / parquet / ns (0.999x ➖, 0↑ 0↓)
File Size Changes (201 files changed, -42.9% overall, 56↑ 145↓)
Totals:
|
Contributor
Benchmarks: Statistical and Population Genetics 📖Commits: PR How to read Verdict and Engines
duckdb / vortex-file-compressed / ns (0.971x ➖, 2↑ 0↓)
duckdb / parquet / ns (1.003x ➖, 0↑ 0↓)
File Size Changes (2 files changed, -37.7% overall, 0↑ 2↓)
Totals:
|
Taking two splits per thread when few splits remain halves effective scan parallelism on compute-bound scans (statpopgen regressed ~1.4x). Only take a second split while enough splits remain to keep every scan thread busy. Signed-off-by: "Joe Isaacs" <joe.isaacs@live.co.uk>
joseph-isaacs
added
action/bench-sql
Claude
github-actions
Bot
removed
the
action/bench-sql
joseph-isaacs
added
the
changelog/performance
Claude
Contributor
Benchmarks: PolarSignals Profiling 📖Commits: PR datafusion / vortex-file-compressed / ns (1.001x ➖, 0↑ 0↓)
No file size changes detected. |
Contributor
Benchmarks: TPC-DS SF=1 on NVME 📖Commits: PR How to read Verdict and Engines
datafusion / vortex-file-compressed / ns (0.996x ➖, 1↑ 1↓)
datafusion / parquet / ns (0.996x ➖, 0↑ 1↓)
duckdb / vortex-file-compressed / ns (1.006x ➖, 2↑ 9↓)
duckdb / parquet / ns (1.007x ➖, 4↑ 7↓)
File Size Changes (25 files changed, -43.9% overall, 0↑ 25↓)
Totals:
|
Contributor
Author
The wins/losses aren't random — they track splits-per-file, and the tail guard means the feature only engages in one suiteTL;DR: every statistically real movement in the SQL benchmarks is in TPC-H SF=10, and it is a win. Everywhere else the 1. Where the numbers actually moveDuckDB /
Only SF=10 has a CI clear of zero and clear of its control. Inside SF=10 the split is not by runtime, it is by which table dominates the scan:
Welch t = -4.87. Biggest wins q17 -10.4%, q18 -10.1%, q06 -9.4%, q19 -8.7%, q07 -6.1%; cold moves further than hot, which Two controls against the obvious alternative explanations:
Clickbench's +1.9% has no internal structure at all — 2. Why: the guard's threshold vs. what the datasets can produceCI runs the SQL suites on I measured what a file actually produces (scratch test writing arrays with the default write strategy and calling
So to clear 256 splits a single file needs roughly ≥16.5M rows of narrow columns, ≥4.1M rows of short strings, or
That matches the measurements exactly: the only suite that can engage is the only suite that moves, and it moves the 3. Consequences / suggestions
Happy to push (1) + (3) onto a branch if you want them measured. Generated by Claude Code |
Contributor
Author
Correction: the bench runs are pinned to one socket, and that makes the guard's thread count half of DuckDB'sI used 128 vCPUs above. Wrong — 1. So on the bench runner 2. With the corrected numbers the fit gets sharper, and the metric is splits per thread, not splits. Using the
3. The suites are more split-starved than I said. TPC-H SF=1 Revised suggestions, in priority order:
Generated by Claude Code |
Contributor
Author
Proof that the thread-count mismatch happens, and a fixBranch: Proof, from DuckDB's own source
idx_t DBConfig::GetSystemMaxThreads(FileSystem &fs) {
idx_t physical_cores = std::thread::hardware_concurrency();
#ifdef __linux__
if (const char *slurm_cpus = getenv("SLURM_CPUS_ON_NODE")) { ... }
return MaxValue<idx_t>(CGroups::GetCPULimit(fs, physical_cores), 1);
#else
return MaxValue<idx_t>(physical_cores, 1);
#endif
}The only two things that can shrink it are Proof, measuredSame box (4 CPUs), pinned to 2, duckdb 1.5.5: Peak OS thread count during a parallel aggregate was identical pinned vs unpinned, i.e. DuckDB really does build the same-sized pool. On the The fix
What I verified, and what I could not
Not addressed here (from the previous comment): the guard is still per-file rather than over the whole scan's remaining split pool, and the split futures are still driven locally rather than spawned on the runtime handle. Happy to do either next. Generated by Claude Code |
Contributor
Author
Correction: I got the crate built and tested after all — the fix passesMy previous comment said The blocker was only the DuckDB source download ( That runs bindgen over the real 1.5.5 headers and compiles and both undefined references match, byte for byte, symbols defined in a real DuckDB 1.5.5 binary: Then I linked the test binary against that same DuckDB 1.5.5 library and ran the suite: That includes the 7 new So the state of the branch is: builds, clippy-clean, formatted, 224/224 tests green locally against DuckDB 1.5.5. The only thing I still cannot do here is measure it — that needs the pinned bench runner. Generated by Claude Code |
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
Each DuckDB scan thread previously took a single split at a time and blocked on its future before exporting, so the IO for the next split only started after the current one was fully consumed. This PR lets each thread take two splits at once and drive them concurrently, overlapping the second split's IO with decode/export of the first.
Changes
reader_try_initialize_scannow pops up toSPLITS_PER_SCAN(2) splits from the file instead of one and wraps them in afuturesstream stored onLocalState.bufferedwhen the scan must preserve file order (afile_row_numbercolumn is projected) andbuffer_unorderedotherwise, so both split futures are polled concurrently while the thread awaits the next result.reader_scanandreader_scan_aggregatepull completed arrays from the stream via a sharednext_split_arrayhelper; when the stream drains, the thread returns toreader_try_initialize_scanfor more splits.orderedflag is computed once inreader_initializeand stored onOpenFileReader, since the FFI signature ofreader_try_initialize_scandoesn't carry the global state.Generated by Claude Code