Repository navigation
[EXPERIMENT] Narrow internal children with encode_values - #10943
connortsui20 wants to merge 2 commits into
Conversation
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
encode_valuesencode_values
Merging this PR will regress 1 benchmark3 warnings
Warning Please fix the performance issues or acknowledge them on CodSpeed. Performance Changes
Tip Investigate this regression by commenting Comparing Footnotes
|
Summary
Widenencoding for integers and decimals #10733Internal integer children now keep their signedness when the compressor narrows them. Each call to the deprecated
encode_valuesmarks a child whose dtype policy still needs a decision.Changes
WidenArray::encode_values, using the sameTODO(connor)reason at each call.i16rather thanu8. Unsigned children keep their behavior.encode_valuestests. Add compact list and string serialization round trips that excludevortex.widen. The list case asserts signed offsets and fails with the old narrowing.API Changes
PrimitiveArrayExt::narrowis removed. Newly written files retain signed dtypes for signed internal children. Encoding IDs and metadata formats do not change.Source audit and signed-child readers
The brief's table contains 24 calls, rather than 25. All 24 calls are migrated. The binary VarBin scheme builds
u64offsets, and OnPair produces unsigned children, so those sites do not change signedness. Dictionary codes, RunEnd ends, RLE indices and offsets, and patch and sparse indices are also unsigned. List and ListView callers can provide signed children. FSST producesi32lengths andi32ori64offsets. DateTimeParts produces signed parts.All locations below are relative to the PR source tree. CUDA results are static inspection only.
vortex-array/src/arrays/list/array.rs:213,:231,:308vortex-array/src/arrays/listview/array.rs:239,:269,:435;vortex-array/src/arrays/listview/rebuild.rs:165vortex-array/src/arrays/varbin/array.rs:238;vortex-array/src/arrays/varbin/vtable/canonical.rs:29encodings/fsst/src/canonical.rs:34,:70,:98;encodings/fsst/src/array.rs:362encodings/datetime-parts/src/canonical.rs:69,:135i32before typed reads.encodings/onpair/src/canonical.rs:42,:73;encodings/onpair/src/decode.rs:51,:66vortex-arrow/src/executor/list.rs:97,:155,:162vortex-arrow/src/executor/byte.rs:108vortex-duckdb/src/exporter/list.rs:91;vortex-duckdb/src/exporter/list_view.rs:129vortex-duckdb/src/exporter/canonical.rs:26,:31vortex-cuda/src/kernel/arrays/list.rs:88;vortex-cuda/kernels/src/list.cu:36;vortex-cuda/kernels/src/list_view.cu:227vortex-cuda/src/kernel/encodings/fsst.rs:158,:202;vortex-cuda/kernels/src/fsst.cu:298vortex-cuda/kernels/src/arrow_offsets.cu:56vortex-cuda/kernels/src/onpair.cu:107Snapshots
compact/list_of_int_runsu16toi16.regular/list_of_int_runsu16toi16.regular/string_fsst_structuredu8toi8, code offsetsu32toi32.regular/string_low_cardinalityu8toi8.regular/struct_mixedu8toi8.Every stored range fits the signed type at the existing width. No byte-count changes need explanation.
Checks and compatibility
vortex-array,vortex-btrblocks,vortex-compressor, andvortex-fastlanes, with all targets, all features, and warnings denied: passed.nightly-2026-09-10for those four crates: passed.cargo nextest run -p vortex-array: 5,189 passed, one skipped.cargo nextest run -p vortex-btrblocks --all-features: 92 passed.cargo nextest run -p vortex-compressor -p vortex-fastlanes: 1,240 passed.cargo nextest run -p vortex-fsst -p vortex-onpair -p vortex-datetime-parts: 242 passed, one skipped.cargo nextest run -p vortex-file: 189 passed. No file-test changes were needed.U16and expectedI16. After migration, both cases passed..narrow(calls remain. All 24 migrated calls have the exact deprecation reason.git diff --checkpassed.0.90.0reader passed 36 fixtures, failed none, and skipped the same seven newer decimal fixtures for both base9bf403a1ceand WD6. The per-fixture results match exactly insubsetmode. All 43 fixtures were generated on each revision. The ClickBench download succeeded, and both runs reused the same deterministic sample. Both temporary compatibility worktrees were removed.b2bd758df8. Its results are pending.Performance evidence and remaining CI work
Local btrblocks comparisons were skipped at the maintainer's request. CodSpeed does not run the btrblocks benchmarks.
The valid WD3 report compares
7dd6ba4with2fb259b. All 12take_mapcases are classifiedNoChange, with latency differences from -1.03% to +0.96%. The valid WD4 report compares419b196with37031b1. All 12 cases areNoChange, with latency differences from -0.31% to +6.34%. WD4 carries a runner-difference warning. Neither report has an unexpected-base condition. WD1's comment uses head75c7f4a, while its final head isa56a48e, so it is excluded as evidence for the final PR.The
take_mapfixture usesu64indices and values directly and calls no narrowing code. The selectedhashbrown 0.17.1default builder usesfoldhash 0.2.0random state, with per-hasher and shared seeds. A fixed fixture RNG does not fix the hash layout. This establishes a possible source of variance, not the cause of the old regressions.WD6's own CodSpeed comparison, any needed flame graph analysis, and develop-history comparison remain pending the PR benchmark run. Only reports with the correct base and head will be used. No
take_mapcode changed.