Alamb/test encoding speed - #9636
Closed
alamb wants to merge 7 commits into
Closed
Conversation
Previously, the column writer accumulated raw definition and repetition levels in `Vec<i16>` sinks (`def_levels_sink` / `rep_levels_sink`) and only RLE-encoded them in bulk at page-flush time. Replace the two sinks with streaming `LevelEncoder` fields. Levels are now encoded incrementally as each `write_batch` call arrives, so only the compact encoded bytes are held in memory at all times. At page flush, the encoder is consumed and its bytes are written directly into the page buffer; a fresh encoder is swapped in for the next page. Signed-off-by: Hippolyte Barraud <hippolyte.barraud@datadoghq.com>
Signed-off-by: Hippolyte Barraud <hippolyte.barraud@datadoghq.com>
Previously, flushing a data page would `mem::replace` each level encoder with a freshly allocated one, consuming the old encoder to get its buffer. This allocated new internal `Vec`s on every page boundary. We now preserve the internal state of the encoder and reuse memory across pages. Signed-off-by: Hippolyte Barraud <hippolyte.barraud@datadoghq.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Contributor
Author
|
The benchmark failure appears to happen on main as well; I will file a follow on ticket |
Contributor
Author
|
The `v1`, `v2`, and `max_buffer_size` functions required knowing the number of values upfront and pre-allocated buffers. All callers have been migrated to the streaming variants (`v1_streaming`, `v2_streaming`), so remove the dead code and switch the last remaining caller in test utils. Signed-off-by: Hippolyte Barraud <hippolyte.barraud@datadoghq.com>
alamb
force-pushed
the
alamb/test_encoding_speed
branch
from
April 1, 2026 16:19
4d2010c to
d55ca60
Compare
This comment has been minimized.
This comment has been minimized.
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing alamb/test_encoding_speed (d55ca60) to 1f07e54 (merge-base) diff File an issue against this benchmark runner |
Contributor
Author
|
run benchmark arrow_writer |
|
🤖 Arrow criterion benchmark running (GKE) | trigger CPU Details (lscpu)Comparing alamb/test_encoding_speed (d55ca60) to 1f07e54 (merge-base) diff File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
|
🤖 Arrow criterion benchmark completed (GKE) | trigger Instance: CPU Details (lscpu)Details
Resource Usagebase (merge-base)
branch
File an issue against this benchmark runner |
Rich-T-kid
pushed a commit
to Rich-T-kid/arrow-rs
that referenced
this pull request
Jun 2, 2026
…ly (apache#9447) # Which issue does this PR close? - Closes apache#9446. - closes apache#9636 # Rationale for this change When writing a Parquet column with very sparse data, `GenericColumnWriter` accumulates unbounded memory for definition and repetition levels. The raw `i16` values are appended into `Vec<i16>` sinks on every `write_batch` call and only RLE-encoded in bulk when a data page is flushed. For a column that is almost entirely nulls, the actual RLE-encoded output can be tiny, yet the intermediate buffer grows linearly with the number of rows. # What changes are included in this PR? Replace the two raw-level `Vec<i16>` sinks (`def_levels_sink` / `rep_levels_sink`) with streaming `LevelEncoder` fields (`def_levels_encoder` / `rep_levels_encoder`). Behavior is the same, but we keep running RLE-encoded state rather than the full list of rows in memory. Existing logic is reused. # Are these changes tested? Yes, all tests passing. Benchmarks show no regression. `list_primitive` benches improved by 3-5%: ``` Benchmarking list_primitive/default: Warming up for 3.0000 s Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 6.1s, enable flat sampling, or reduce sample count to 60. list_primitive/default time: [1.2109 ms 1.2171 ms 1.2248 ms] thrpt: [1.6999 GiB/s 1.7105 GiB/s 1.7194 GiB/s] change: time: [−3.7197% −2.8848% −2.0036%] (p = 0.00 < 0.05) thrpt: [+2.0445% +2.9705% +3.8634%] Performance has improved. Found 4 outliers among 100 measurements (4.00%) 3 (3.00%) high mild 1 (1.00%) high severe Benchmarking list_primitive/bloom_filter: Warming up for 3.0000 s Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 7.5s, enable flat sampling, or reduce sample count to 50. list_primitive/bloom_filter time: [1.4405 ms 1.4810 ms 1.5292 ms] thrpt: [1.3615 GiB/s 1.4058 GiB/s 1.4452 GiB/s] change: time: [−6.4332% −4.7568% −2.9048%] (p = 0.00 < 0.05) thrpt: [+2.9917% +4.9944% +6.8755%] Performance has improved. Found 5 outliers among 100 measurements (5.00%) 2 (2.00%) high mild 3 (3.00%) high severe Benchmarking list_primitive/parquet_2: Warming up for 3.0000 s Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 6.3s, enable flat sampling, or reduce sample count to 60. list_primitive/parquet_2 time: [1.2271 ms 1.2311 ms 1.2362 ms] thrpt: [1.6841 GiB/s 1.6911 GiB/s 1.6966 GiB/s] change: time: [−5.8536% −4.9672% −4.1905%] (p = 0.00 < 0.05) thrpt: [+4.3738% +5.2269% +6.2175%] Performance has improved. Found 5 outliers among 100 measurements (5.00%) 2 (2.00%) high mild 3 (3.00%) high severe list_primitive/zstd time: [2.0056 ms 2.0148 ms 2.0262 ms] thrpt: [1.0275 GiB/s 1.0333 GiB/s 1.0381 GiB/s] change: time: [−4.7073% −3.6719% −2.6698%] (p = 0.00 < 0.05) thrpt: [+2.7431% +3.8118% +4.9398%] Performance has improved. Found 12 outliers among 100 measurements (12.00%) 2 (2.00%) high mild 10 (10.00%) high severe list_primitive/zstd_parquet_2 time: [2.0455 ms 2.0730 ms 2.1120 ms] thrpt: [1009.4 MiB/s 1.0043 GiB/s 1.0178 GiB/s] change: time: [−5.8626% −3.7672% −1.4196%] (p = 0.00 < 0.05) thrpt: [+1.4401% +3.9146% +6.2277%] Performance has improved. Found 7 outliers among 100 measurements (7.00%) 2 (2.00%) high mild 5 (5.00%) high severe Benchmarking list_primitive_non_null/default: Warming up for 3.0000 s Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 6.6s, enable flat sampling, or reduce sample count to 60. list_primitive_non_null/default time: [1.3199 ms 1.3333 ms 1.3504 ms] thrpt: [1.5384 GiB/s 1.5581 GiB/s 1.5740 GiB/s] change: time: [−4.1662% −2.3491% −0.7148%] (p = 0.01 < 0.05) thrpt: [+0.7200% +2.4056% +4.3473%] Change within noise threshold. Found 6 outliers among 100 measurements (6.00%) 3 (3.00%) high mild 3 (3.00%) high severe Benchmarking list_primitive_non_null/bloom_filter: Warming up for 3.0000 s Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 8.4s, enable flat sampling, or reduce sample count to 50. list_primitive_non_null/bloom_filter time: [1.6567 ms 1.6668 ms 1.6805 ms] thrpt: [1.2362 GiB/s 1.2464 GiB/s 1.2540 GiB/s] change: time: [−2.7884% −1.3493% +0.2820%] (p = 0.07 > 0.05) thrpt: [−0.2812% +1.3677% +2.8684%] No change in performance detected. Found 4 outliers among 100 measurements (4.00%) 1 (1.00%) high mild 3 (3.00%) high severe Benchmarking list_primitive_non_null/parquet_2: Warming up for 3.0000 s Warning: Unable to complete 100 samples in 5.0s. You may wish to increase target time to 7.2s, enable flat sampling, or reduce sample count to 50. list_primitive_non_null/parquet_2 time: [1.4279 ms 1.4409 ms 1.4551 ms] thrpt: [1.4277 GiB/s 1.4418 GiB/s 1.4550 GiB/s] change: time: [−2.0598% −0.9952% −0.1318%] (p = 0.04 < 0.05) thrpt: [+0.1319% +1.0052% +2.1032%] Change within noise threshold. Found 3 outliers among 100 measurements (3.00%) 2 (2.00%) high mild 1 (1.00%) high severe list_primitive_non_null/zstd time: [2.6966 ms 2.7358 ms 2.7994 ms] thrpt: [759.93 MiB/s 777.60 MiB/s 788.89 MiB/s] change: time: [−3.8379% −2.1418% +0.0785%] (p = 0.03 < 0.05) thrpt: [−0.0784% +2.1887% +3.9911%] Change within noise threshold. Found 7 outliers among 100 measurements (7.00%) 3 (3.00%) high mild 4 (4.00%) high severe list_primitive_non_null/zstd_parquet_2 time: [2.7684 ms 2.7861 ms 2.8099 ms] thrpt: [757.07 MiB/s 763.55 MiB/s 768.44 MiB/s] change: time: [−6.4460% −4.1387% −2.1474%] (p = 0.00 < 0.05) thrpt: [+2.1946% +4.3174% +6.8901%] Performance has improved. ``` # Are there any user-facing changes? None. Some internal symbols are now unused. I added some `#[allow(dead_code)]` statements since these were experimental-visible and might be externally relied on. --------- Signed-off-by: Hippolyte Barraud <hippolyte.barraud@datadoghq.com> Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
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
3 participants
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.
This is a dummy PR that has all the changes in that I can automatically run the benchmarks on
The benchmark runner currently doesn't support branches called
main