Uh oh!
There was an error while loading. Please reload this page.
[opt](csv reader) optimize stream load CSV read performance - #60920
Closed
liaoxin01 wants to merge 4 commits into
Closed
[opt](csv reader) optimize stream load CSV read performance#60920liaoxin01 wants to merge 4 commits into
liaoxin01 wants to merge 4 commits into
Conversation
Cache nullable string column pointers per-batch to eliminate per-row assert_cast, inline the write path to bypass StringSerDe layer, and pre-reserve ColumnStr/NullMap capacity to reduce realloc overhead.
Thearas
commented
Feb 28, 2026
Contributor
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
liaoxin01force-pushed
the
opt/csv-reader-stream-load-perf
branch
from
February 28, 2026 15:01
b5643ff to
9711a75CompareContributor
There was a problem hiding this comment.
Pull request overview
Optimizes vectorized CSV stream-load parsing for nullable string columns by removing per-row SerDe overhead and reducing reallocations in the hot loop.
Changes:
- Adds per-batch caching of
ColumnNullablenested string column and null-map pointers to avoid repeatedassert_castper row. - Inlines the nullable-string CSV decode path (null detection + escape handling +
insert_data/push_back) instead of calling through SerDe layers. - Pre-reserves
offsets,chars, andnull_mapcapacity per batch to reducePODArraygrowth overhead.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| be/src/vec/exec/format/csv/csv_reader.h | Adds nullable string column cache structures/members and required column includes. |
| be/src/vec/exec/format/csv/csv_reader.cpp | Initializes/uses the cache per batch, inlines nullable-string deserialization, and adds per-batch reserves. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
liaoxin01
commented
Feb 28, 2026
ContributorAuthor
run buildall |
doris-robot
commented
Feb 28, 2026
TPC-H: Total hot run time: 28649 ms |
doris-robot
commented
Feb 28, 2026
TPC-DS: Total hot run time: 184062 ms |
hello-stephen
commented
Feb 28, 2026
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Feb 28, 2026
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
PR approved by at least one committer and no changes requested. |
Contributor
PR approved by anyone and no changes requested. |
hello-stephen
commented
Mar 6, 2026
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Resolved conflicts by applying CSV reader optimization to new file location after BE code structure refactor (be/src/vec/exec/format/csv/ -> be/src/format/csv/). The optimization includes: - Cache nullable string column pointers per-batch - Inline nullable string write path to bypass SerDe layer - Pre-reserve column capacity to reduce realloc overhead
- Add DCHECK bounds check before cache access for safety - Add comment explaining 64-byte string length estimate - Add comment for code structure in else block - Remove unused data_type_string_serde.h include
liaoxin01
commented
Mar 11, 2026
ContributorAuthor
/review |
Contributor
There was a problem hiding this comment.
Code Review Summary
This PR optimizes CSV stream load performance for nullable string columns by caching column pointers per-batch and inlining the nullable string write path. The optimization approach is sound and the semantic equivalence with the original _deserialize_nullable_string is mostly correct. However, there are issues that should be addressed.
Critical Checkpoint Conclusions
Goal & Correctness: The goal is to eliminate per-row overhead from assert_cast and StringSerDe abstraction. The inlined fast path is semantically equivalent to the original _deserialize_nullable_string for the success path. The original method's error-on-deserialization fallback (filling null on failure) is dead code for string types since DataTypeStringSerDeBase::deserialize_one_cell_from_csv always returns Status::OK(), so dropping it is correct.
Minimal & Focused: The change is focused on a single optimization. However, there is dead code left behind (see Issue 1).
Concurrency: No concurrency issues. CsvReader is single-threaded per scan range.
Memory Safety: The raw pointer caching pattern is safe within a single batch since the column pointers remain stable after mutate_columns() and before set_columns(). Block::mutate_columns() does not null out the block's column pointers, so the !_is_load path accessing block->get_by_position() is safe. The reserve() calls use reasonable estimates and prevent realloc during the row loop.
Parallel Code Paths:_deserialize_nullable_string is a virtual method also used via HiveCsvReader. The optimization only applies to the _fill_dest_columns hot path in CsvReader which is appropriate.
Configuration Items: No new configuration items added. None needed.
Incompatible Changes: None - this is a purely internal optimization.
Test Coverage: No tests added. While this is a pure performance optimization that should be behaviorally equivalent, the inlined logic is non-trivial (null checks, escape handling, trim_double_quotes side effects). At minimum, existing regression tests for CSV stream load should cover this, but no new targeted tests are included.
Observability: No observability changes needed for this optimization.
Performance: The optimization is well-motivated by flame graph analysis. However, the dead col_ptr computation (Issue 1) partially undermines the per-row cost savings.
Issues Found
See inline comments for details.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Remove dead col_ptr computation that was executed on every row - Fix C-style cast to use std::max<size_t> template parameter
liaoxin01
commented
Mar 11, 2026
ContributorAuthor
run buildall |
doris-robot
commented
Mar 11, 2026
TPC-H: Total hot run time: 27594 ms |
doris-robot
commented
Mar 11, 2026
TPC-DS: Total hot run time: 153187 ms |
hello-stephen
commented
Mar 11, 2026
Contributor
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
hello-stephen
commented
Mar 11, 2026
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
16 tasks
liaoxin01 added a commit
that referenced
this pull request
Jun 13, 2026
…t load hot path (#64476) ### What problem does this PR solve? Issue Number: close #xxx Related PR: #60920 (previous attempt, superseded by this stateless implementation) Problem Summary: When loading CSV data, every column is read as a nullable string, so `_deserialize_nullable_string` is the per-row per-column hot path (ClickBench: 105 columns x 100M rows = ~10.5 billion cells). Flame graph shows two major per-cell overheads: 1. `assert_cast<ColumnNullable&>` performs a typeid comparison per cell in release builds. 2. `DataTypeStringSerDe::deserialize_one_cell_from_csv` adds a call layer with another per-cell `assert_cast<ColumnString&>` inside, plus Status plumbing. Its fill-null-on-failure branch is dead code since the method never fails. ### Changes 1. Use `assert_cast<..., TypeCheckOnRelease::DISABLE>` in `CsvReader::_deserialize_nullable_string` and `TextReader::_deserialize_nullable_string`, which compiles to a plain `static_cast` in release builds. Debug builds still verify the cast. 2. Write the string column and null map directly instead of going through the SerDe layer (semantically identical, verified against `ColumnNullable::insert_data` / `DataTypeStringSerDe` implementations). The virtual `_deserialize_nullable_string` dispatch is kept, so TextReader's hive-text semantics (different escape handling and null detection) remain intact. 3. Add `_reserve_nullable_string_columns`, called once per batch: it performs checked `assert_cast`s (backing the unchecked per-row casts with a real type validation per batch, throwing instead of UB on mismatch) and reserves offsets/null_map capacity to avoid incremental PODArray growth in the row loop. The implementation is stateless: no cached column pointers, no per-batch member state to initialize/clear. ### Performance A/B test on full ClickBench dataset (73GB / 100M rows / 105 columns), identical deployment and config, only the BE binary differs: | Metric | Before | After | Improvement | |---|---|---|---| | Total load time (BE LoadTime) | 636.6s | 530.9s | -16.6% (1.20x) | | CSV parse (ReadDataTime) | 590.6s | 484.5s | -18.0% | | Avg throughput | 115 MB/s | 138 MB/s | +20% | All 10 splits (10M rows each) improved consistently by 14-18% with small variance. Loaded row counts are identical between the two runs (99,997,497 rows).
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 freeto 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.
Proposed changes
Optimize stream load CSV read performance for nullable string columns by eliminating per-row overhead from the SerDe abstraction layer.
Changes
Cache nullable string column pointers per-batch: Pre-compute
assert_castresults (ColumnStr and NullMap pointers) once per batch instead of once per row per column, stored inNullableStringColumnCache.Inline nullable string write path: Bypass
_deserialize_nullable_stringandStringSerDe::deserialize_one_cell_from_csvin the hot loop, directly performing null checks, escape handling, andinsert_data/push_back.Pre-reserve column capacity: Reserve
offsets,chars, andnull_mapcapacity at batch start to reduce PODArray realloc overhead during the row loop.Performance
Tested with ClickBench dataset stream load:
Flame graph analysis
Before optimization,
_deserialize_nullable_stringpath dominated with +96s self-time from:assert_cast<ColumnNullable&>(+65s)StringSerDe::deserialize_one_cell_from_csvintermediate layer (+54s)After optimization, these costs are eliminated or amortized to per-batch.