Repository navigation
Conversation
Signed-off-by: Matt Katz <[email protected]>
Merging this PR will degrade performance by 13.28%
|
| Mode | Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|---|
| ❌ | Simulation | density_sweep_single_slice[0.9] |
32.1 µs | 48.3 µs | -33.57% |
| ❌ | Simulation | sum_v2_i64 |
193.9 µs | 224.7 µs | -13.7% |
| ❌ | Simulation | sum_i64 |
194 µs | 224.4 µs | -13.56% |
| ❌ | WallTime | dict_canonicalize_gt_u8_neon[1000000] |
488.7 µs | 547.2 µs | -10.68% |
| ⚡ | WallTime | compare_u64_avx2 |
4.1 µs | 3.7 µs | +10.8% |
Tip
Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.
Comparing mk/standalone-array-plugins (e50242a) with develop (292cf6f)
Footnotes
-
503 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. ↩
Signed-off-by: Matt Katz <[email protected]>
## Summary Removes the blanket `impl<V: VTable> ArrayPlugin for V` and the `serialize` / `deserialize` methods on `VTable`. Every serializable encoding now implements `ArrayPlugin` explicitly on its vtable type, in a `plugin.rs` child module next to its `impl VTable`. Redo of #10111 on current develop. ## Changes - `VTable` covers only in-memory behaviour; serde is `impl ArrayPlugin for X`. - Serde bodies are ported verbatim, so wire formats are unchanged. - `read_ids` / `write_ids` default to `vec![self.id()]`, what the blanket impl declared. Plugins with one wire ID no longer spell them out, including `ALPPatchedPlugin`, `ScalarFnArrayPlugin` and the Python stub. - Inside an impl the wire ID is `ArrayPlugin::id(self)`; only the `id` method itself delegates to `VTable::id`. - `session.arrays().register(X)` is unchanged for every encoding that was serializable. - In-memory-only vtables lose the two stub methods and get no plugin: Filter, Slice, Interleave, ScalarFn, ScalarFnValidity, ForeignArray, PiecewiseSequence, Widen, Shared, test-harness vtables. - `DecimalPlugin` (#10995) becomes `impl ArrayPlugin for Decimal`: one wire ID, same legacy-node reads, no separate type. Its module, `StoredDecimal` and tests stay where they are. - Multi-ID plugins (`BitPackedPlugin`, `FoRPlugin`, `DecimalBytePartsPlugin`) and `WidenPlugin` are untouched; `ALPPatchedPlugin` now delegates to `ArrayPlugin::deserialize(&ALP, ..)`. - `PythonVTable` keeps a stub plugin so the Python `register` API still compiles. - Vtable files that gained a `plugin.rs` child are directory modules (`array.rs` becomes `array/mod.rs`, 14 pure renames). ALP's own impl and `ALPPatchedPlugin` share `alp/array/plugin.rs`. - Tests that called the vtable serde directly (ALP, ALPRD, Dict, List, RunEnd, VarBin, RLE, serde.rs, unary, zstd buffers) go through `ArrayPlugin` instead. | | count | |---|---| | new `impl ArrayPlugin` (vortex-array) | 20 | | new `impl ArrayPlugin` (encodings + python) | 17 | | vtables that only lose the stub methods | 14 | ## API changes - `VTable` loses `serialize` and `deserialize`. An external vtable that was registered through the blanket impl must now `impl ArrayPlugin` to be registrable. - `ArrayPlugin::read_ids` / `write_ids` gain defaults of `vec![self.id()]`. - `vortex_array::arrays::DecimalPlugin` is removed; register `Decimal` instead. ## Behaviour changes - `PiecewiseSequence` is no longer registered by `ArraySession::default()`; it never serialized, and its error is now "is not registered for serialization". - The writer seeds the footer's encoding list from the session registry, so dropping `vortex.piecewise-sequence` makes every default-session file 48 bytes smaller. `test_stock_ticker_file_size` expectations move accordingly (70060 → 70012, 69996 → 69948, 55144 → 55096). - `DecimalByteParts`, `BitPacked` and `FoR` can no longer be registered bare, so `bare_vtable_refuses_serde` and `vtable_serde_requires_plugin` are removed. ## Testing - `cargo clippy --all-targets --all-features -- -D warnings` and `cargo +nightly-2026-09-10 fmt --check` on every touched crate (vortex-array, vortex-file, vortex-layout, vortex-python and the 13 encodings crates). - `cargo nextest run` on all touched crates plus vortex-ipc and vortex-scan: 8981 passed, 0 failed; `cargo test --doc` on vortex-array, fastlanes, decimal-byte-parts and alp. - `cargo nextest run -p vortex-file -E 'test(test_stock_ticker_file_size)'` with and without `--features zstd`, to derive the new sizes. ## Follow-up - Pre-existing, unchanged here: the Patched, RLE, OnPair, ALPRD, FSST and Zstd deserializers index children or buffers before checking their counts, so a malformed node panics in `SerializedArray::child` instead of returning an error. Worth hoisting the count checks in a separate PR. - Fold `FoRPlugin` / `BitPackedPlugin` / `DecimalBytePartsPlugin` into `impl ArrayPlugin for FoR` etc. if wanted. - Overlaps with #11298, which makes the same change with inline impls and trait defaults for `read_ids` / `write_ids`. --------- Signed-off-by: Matt Katz <[email protected]>
Summary
impl<V: VTable> ArrayPlugin for V.serialize/deserializefrom theVTabletrait. The vtable now only covers in-memory behaviour.ArrayPlugindirectly on its vtable type (impl ArrayPlugin for Primitive,impl ArrayPlugin for FoR, ...). There are no new plugin structs, andsession.arrays().register(X)is unchanged.plugin.rschild module of the file that holdsimpl VTable for X, e.g.primitive/vtable/plugin.rsoralp/array/plugin.rs. Because it is a child module, it can use the parent's private metadata types without visibility changes.Encodings that were never serializable lose the two methods and get no plugin: Filter, Slice, Shared, Interleave, ScalarFn, ForeignArray, TransposedBool, PiecewiseSequence and the test-harness vtables.
DecimalBytePartsPluginstruct is removed. Its v1/v2 serde now lives inimpl ArrayPlugin for DecimalByteParts, andinitializeregistersDecimalByteParts. Thebare_vtable_refuses_serdetest is deleted because its premise no longer holds.ALPPatchedPluginandBitPackedPatchedPluginare unchanged apart from delegating toArrayPlugin::deserialize(&ALP/&BitPacked, ...).Behaviour changes
PiecewiseSequenceis no longer registered inArraySession::default(), andTransposedBoolis no longer registered byvortex_fastlanes::initialize. Neither could serialize or deserialize before. Serializing them now fails with "not registered for serialization" instead of "does not support serialization", andserialization_is_not_supportedis updated to match.PythonVTablegets a minimalArrayPluginwhose serialize returnsOk(None)and whose deserialize bails, the same as before. This keepsvortex._lib.registry.registerworking.