Add ArrayPlugin to enable serde for new DecimalBytePartsArray format - #9810
Conversation
Merging this PR will degrade performance by 19.87%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | column_x_constant[2] |
131.3 µs | 454.3 µs | -71.09% |
| ❌ | Simulation | column_x_constant[32] |
132 µs | 390.1 µs | -66.16% |
| ❌ | Simulation | column_x_column[2] |
101.8 µs | 203.1 µs | -49.85% |
| ❌ | Simulation | column_x_constant[256] |
311.9 µs | 590.3 µs | -47.17% |
| ❌ | Simulation | nullable[2] |
324.1 µs | 591 µs | -45.17% |
| ❌ | Simulation | non_nullable[2] |
327.9 µs | 593.2 µs | -44.73% |
| ❌ | Simulation | column_x_extension_constant[2] |
147.1 µs | 246.9 µs | -40.41% |
| ❌ | Simulation | nullable[2] |
416.8 µs | 629.4 µs | -33.78% |
| ❌ | Simulation | non_nullable[2] |
418.9 µs | 630.4 µs | -33.55% |
| ❌ | Simulation | column_x_extension_constant[256] |
277.8 µs | 415.2 µs | -33.09% |
| ❌ | Simulation | random_i8[0.8] |
72.9 µs | 98.1 µs | -25.76% |
| ❌ | Simulation | column_x_extension_constant[32] |
135.2 µs | 167.7 µs | -19.39% |
| ❌ | Simulation | column_x_column[32] |
76.8 µs | 94.1 µs | -18.38% |
| ❌ | Simulation | decompress[datetime_for_bp] |
159.3 µs | 193.6 µs | -17.68% |
| ❌ | WallTime | arrow_checked_add_u32_avx2[16384] |
17.6 µs | 21.3 µs | -17.21% |
| ❌ | Simulation | column_x_column[256] |
75.2 µs | 88.5 µs | -15.04% |
| ❌ | Simulation | optimize_lookup_predicate[ids=16, shape=balanced_or] |
70 µs | 80.9 µs | -13.45% |
| ❌ | WallTime | dbp_split_kernel_mixed_null_neon[(I256, 1024)] |
1.9 µs | 2.2 µs | -12.44% |
| ⚡ | Simulation | random_i16[0.95] |
98.5 µs | 76 µs | +29.6% |
| ⚡ | Simulation | random_i8[0.01] |
45.3 µs | 39.7 µs | +14.22% |
| ... | ... | ... | ... | ... | ... |
ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing mk/dbp-serde (8ba61cc) with mk/dbp-v2-feature (8fc58e0)
Footnotes
-
229 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. ↩
b822fb6 to
e13f457
Compare
a68b2ae to
b970a8f
Compare
ArrayPlugin to enable serde for new DecimalBytePartsArray format
03a13f8 to
72f2396
Compare
72f2396 to
d846ec7
Compare
| const MAX_PRECISION_38: i128 = 99_999_999_999_999_999_999_999_999_999_999_999_999; | ||
|
|
||
| /// The largest unscaled value a `Decimal(76, _)` can hold: `10^76 - 1`. | ||
| fn max_precision_76() -> i256 { | ||
| i256::from_i128(10).wrapping_pow(76) - i256::ONE | ||
| } | ||
|
|
||
| /// Values that exercise every 64-bit window of an `i128`, both signs, and the boundaries | ||
| /// where a lower part carries into the MSP. | ||
| pub(crate) fn wide_i128_values() -> Vec<i128> { | ||
| vec![ | ||
| 0, | ||
| 1, | ||
| -1, | ||
| (1 << 64) - 1, | ||
| 1 << 64, | ||
| -(1 << 64), | ||
| -((1 << 64) + 1), | ||
| MAX_PRECISION_38, | ||
| -MAX_PRECISION_38, | ||
| 1 << 100, | ||
| ] | ||
| } | ||
|
|
||
| /// Values that exercise every 64-bit window of an `i256`. | ||
| pub(crate) fn wide_i256_values() -> Vec<i256> { | ||
| vec![ | ||
| i256::ZERO, | ||
| i256::ONE, | ||
| i256::ZERO - i256::ONE, | ||
| i256_of(0, u128::MAX), | ||
| i256_of(1, 0), | ||
| i256_of(-1, 0), | ||
| i256_of(-1, u128::MAX - 1), | ||
| i256_of(1 << 64, 12345), | ||
| max_precision_76(), | ||
| i256::ZERO - max_precision_76(), | ||
| ] | ||
| } |
Use one ArrayPlugin for the frozen single-part format and the new wide format. Preserve frozen files with wider physical storage and add wire contract tests plus an opt-in compatibility fixture. Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Move the plugin and serde coverage into plugin.rs while preserving metadata and frozen-format VTable serde. Share wide decimal test fixtures and exercise frozen compatibility through both registration paths. Include the v2 compatibility fixture in the default suite without enabling unstable encodings. Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
d846ec7 to
46de82a
Compare
| pub fn decimal_byte_parts_v2_id() -> ArrayId { | ||
| static ID: CachedId = CachedId::new("vortex.decimal_byte_parts_v2"); | ||
| *ID |
There was a problem hiding this comment.
I thought the new array in mem would have v2? So we would have v1 == "" here
| pub fn decimal_byte_parts_v2_id() -> ArrayId { | |
| static ID: CachedId = CachedId::new("vortex.decimal_byte_parts_v2"); | |
| *ID | |
| pub fn decimal_byte_parts_v2_id() -> ArrayId { | |
| static ID: CachedId = CachedId::new("vortex.decimal_byte_parts.v2"); | |
| *ID |
Keep one plugin dispatcher with separate v1 and v2 metadata and serde functions operating directly on the current DBP array. Preserve the current draft, including the latest metadata names and validation edits, for comparison against an ArrayRepresentation prototype. Snapshot validation: 272 tests passed and 3 failed before fail-fast cancelled the remaining 22 tests. The failures cover accepting zero lower parts in v2 and two expectations of the previous error wording. Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
1389183 to
8ba61cc
Compare
| "v2 carries at most {MAX_LOWER_PARTS} lower parts, got {lower_part_count}" | ||
| ); | ||
| vortex_ensure!( | ||
| parts.children.len() == 1 + lower_part_count, |
There was a problem hiding this comment.
we really need to remove children from the vtable and only use slots
…mat (#9810) Add serialization for `DecimalBytePartsArray` with lower parts while preserving the frozen v1 file format. `DecimalBytePartsPlugin` owns both formats and selects the serialized ID from the array's layout: | Array layout | Serialized ID | | --- | --- | | Signed most significant part (MSP) only | `vortex.decimal_byte_parts` | | Signed MSP plus one to three unsigned lower parts | `vortex.decimal_byte_parts_v2` | Both formats deserialize into the same in-memory array representation. Each lower part represents a 64-bit window, but its storage dtype may be narrowed to `u8`, `u16`, or `u32` when its values fit. - Record the MSP's physical integer type, lower-part count, and each lower part's physical integer type in metadata. Deserialization validates these types and restores each child with its recorded dtype. - Keep the frozen format restricted to zero lower parts and require at least one for v2. Frozen metadata remains byte-identical. - Make DBP's VTable `serialize` and `deserialize` methods return errors directing callers to `DecimalBytePartsPlugin`. Keep DBP metadata and serde helpers in `plugin.rs` so the dedicated plugin owns both formats. - Add a separate v2 compatibility fixture covering positive and negative wide `i128` values and nullable `i256` values. The frozen fixture is unchanged. **Breaking:** directly registering `DecimalByteParts`, or calling its VTable serde methods, now errors for both serialization and deserialization, including the frozen v1 format. Replace: ```rust session.arrays().register(DecimalByteParts); ``` with: ```rust session.arrays().register(DecimalBytePartsPlugin); ``` `vortex_decimal_byte_parts::initialize(&session)` already registers the plugin. Existing v1 files remain readable through it; no file migration is required. --------- Signed-off-by: Matt Katz <mhkatz97@gmail.com>
…mat (#9810) Add serialization for `DecimalBytePartsArray` with lower parts while preserving the frozen v1 file format. `DecimalBytePartsPlugin` owns both formats and selects the serialized ID from the array's layout: | Array layout | Serialized ID | | --- | --- | | Signed most significant part (MSP) only | `vortex.decimal_byte_parts` | | Signed MSP plus one to three unsigned lower parts | `vortex.decimal_byte_parts_v2` | Both formats deserialize into the same in-memory array representation. Each lower part represents a 64-bit window, but its storage dtype may be narrowed to `u8`, `u16`, or `u32` when its values fit. - Record the MSP's physical integer type, lower-part count, and each lower part's physical integer type in metadata. Deserialization validates these types and restores each child with its recorded dtype. - Keep the frozen format restricted to zero lower parts and require at least one for v2. Frozen metadata remains byte-identical. - Make DBP's VTable `serialize` and `deserialize` methods return errors directing callers to `DecimalBytePartsPlugin`. Keep DBP metadata and serde helpers in `plugin.rs` so the dedicated plugin owns both formats. - Add a separate v2 compatibility fixture covering positive and negative wide `i128` values and nullable `i256` values. The frozen fixture is unchanged. **Breaking:** directly registering `DecimalByteParts`, or calling its VTable serde methods, now errors for both serialization and deserialization, including the frozen v1 format. Replace: ```rust session.arrays().register(DecimalByteParts); ``` with: ```rust session.arrays().register(DecimalBytePartsPlugin); ``` `vortex_decimal_byte_parts::initialize(&session)` already registers the plugin. Existing v1 files remain readable through it; no file migration is required. --------- Signed-off-by: Matt Katz <mhkatz97@gmail.com>
…mat (#9810) Add serialization for `DecimalBytePartsArray` with lower parts while preserving the frozen v1 file format. `DecimalBytePartsPlugin` owns both formats and selects the serialized ID from the array's layout: | Array layout | Serialized ID | | --- | --- | | Signed most significant part (MSP) only | `vortex.decimal_byte_parts` | | Signed MSP plus one to three unsigned lower parts | `vortex.decimal_byte_parts_v2` | Both formats deserialize into the same in-memory array representation. Each lower part represents a 64-bit window, but its storage dtype may be narrowed to `u8`, `u16`, or `u32` when its values fit. - Record the MSP's physical integer type, lower-part count, and each lower part's physical integer type in metadata. Deserialization validates these types and restores each child with its recorded dtype. - Keep the frozen format restricted to zero lower parts and require at least one for v2. Frozen metadata remains byte-identical. - Make DBP's VTable `serialize` and `deserialize` methods return errors directing callers to `DecimalBytePartsPlugin`. Keep DBP metadata and serde helpers in `plugin.rs` so the dedicated plugin owns both formats. - Add a separate v2 compatibility fixture covering positive and negative wide `i128` values and nullable `i256` values. The frozen fixture is unchanged. **Breaking:** directly registering `DecimalByteParts`, or calling its VTable serde methods, now errors for both serialization and deserialization, including the frozen v1 format. Replace: ```rust session.arrays().register(DecimalByteParts); ``` with: ```rust session.arrays().register(DecimalBytePartsPlugin); ``` `vortex_decimal_byte_parts::initialize(&session)` already registers the plugin. Existing v1 files remain readable through it; no file migration is required. --------- Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Summary
Add serialization for
DecimalBytePartsArraywith lower parts while preserving the frozen v1 file format.DecimalBytePartsPluginowns both formats and selects the serialized ID from the array's layout:vortex.decimal_byte_partsvortex.decimal_byte_parts_v2Both formats deserialize into the same in-memory array representation. Each lower part represents a 64-bit window, but its storage dtype may be narrowed to
u8,u16, oru32when its values fit.Changes
serializeanddeserializemethods return errors directing callers toDecimalBytePartsPlugin. Keep DBP metadata and serde helpers inplugin.rsso the dedicated plugin owns both formats.i128values and nullablei256values. The frozen fixture is unchanged.API Changes
Breaking: directly registering
DecimalByteParts, or calling its VTable serde methods, now errors for both serialization and deserialization, including the frozen v1 format. Replace:with:
vortex_decimal_byte_parts::initialize(&session)already registers the plugin. Existing v1 files remain readable through it; no file migration is required.