Skip to content

Tell the compressor which serialized IDs the writer may emit - #9770

Open
mhk197 wants to merge 6 commits into
mk/dbp-serdefrom
mk/compressor-serialized-ids
Open

Tell the compressor which serialized IDs the writer may emit#9770
mhk197 wants to merge 6 commits into
mk/dbp-serdefrom
mk/compressor-serialized-ids

Conversation

@mhk197

@mhk197 mhk197 commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

Filtering compression schemes by in-memory encoding cannot distinguish multiple wire formats of the same encoding. In order to support encodings like DecimalByteParts with multiple wire formats, the compressor must know which wire formats are enabled and must be able to select the appropriate schemes.

The writer now passes its permitted serialized IDs to compressor configuration, which selects the newest compatible version of each scheme before matching, generating statistics, estimating, or compressing.

Scheme::produced_encodings declares the serialized IDs that a scheme directly introduces. Every declared ID must be allowed.

Alternative versions are represented by Scheme::predecessor: register only the newest version, fall back through its predecessor chain when its IDs are unavailable, and remove the chain when no version is eligible. Historical versions do not compete with their replacements during compression.

BtrBlocksCompressorBuilder::allow_serialized_ids replaces retain_allowed_encodings and applies restrictions at build time, including to schemes added after the restriction. Both it and CascadingCompressor::with_allowed_serialized_ids intersect repeated restrictions. Registration order is preserved, and scheme IDs in a predecessor chain resolve to the selected version for exclusions and has_scheme checks. Cycles and overlapping chains are rejected.

The default file writer uses the serialized IDs of its enabled editions, or every registered serialized ID when edition enforcement is disabled. Custom write strategies retain their own configuration. Serialization still chooses the oldest wire form the resulting array fits and validates it against the writer's allowed IDs.

Existing production schemes have no predecessors, so this prepares version selection for consumers such as bitpacking (#9754) and decimal byte parts (#9759). The related edition specification is discussed in #9779.

Tests cover version selection before estimation, required output IDs, repeated restrictions, schemes added after restrictions, registration order, exclusions across versions, invalid predecessor chains, and writer round trips with unavailable encodings.

@codspeed-hq

codspeed-hq Bot commented Sep 4, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 11.21%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 2 improved benchmarks
❌ 4 regressed benchmarks
✅ 2181 untouched benchmarks
🆕 18 new benchmarks
⏩ 228 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
WallTime arrow_checked_add_u32_neon[16384] 13.3 µs 20.4 µs -34.7%
Simulation decompress[u64, (4000, 1024)] 70.8 µs 87 µs -18.61%
Simulation decompress[datetime_for_bp] 160.1 µs 193.8 µs -17.39%
WallTime mul_u32_nonnull_avx512 5.6 µs 6.3 µs -11.61%
Simulation chunked_bool_canonical_into[(1000, 10)] 30.6 µs 27.2 µs +12.69%
WallTime filtered_owned_i64_avx512[OneNullInEight] 26.2 µs 23.4 µs +12%
🆕 Simulation dbp_assemble[(I128, 1024)] N/A 48.3 µs N/A
🆕 Simulation dbp_assemble[(I128, 8192)] N/A 267 µs N/A
🆕 Simulation dbp_assemble[(I256, 1024)] N/A 104 µs N/A
🆕 Simulation dbp_assemble[(I256, 8192)] N/A 719.3 µs N/A
🆕 Simulation dbp_assemble[(I64, 1024)] N/A 9.3 µs N/A
🆕 Simulation dbp_assemble[(I64, 8192)] N/A 7.5 µs N/A
🆕 Simulation dbp_split_all_valid[(I128, 1024)] N/A 52.2 µs N/A
🆕 Simulation dbp_split_all_valid[(I128, 8192)] N/A 288.4 µs N/A
🆕 Simulation dbp_split_all_valid[(I256, 1024)] N/A 89.7 µs N/A
🆕 Simulation dbp_split_all_valid[(I256, 8192)] N/A 550.5 µs N/A
🆕 Simulation dbp_split_all_valid[(I64, 1024)] N/A 7.8 µs N/A
🆕 Simulation dbp_split_all_valid[(I64, 8192)] N/A 7.8 µs N/A
🆕 Simulation dbp_split_mixed_null[(I128, 1024)] N/A 73.7 µs N/A
🆕 Simulation dbp_split_mixed_null[(I128, 8192)] N/A 318.1 µs N/A
... ... ... ... ... ...

ℹ️ 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/compressor-serialized-ids (bf0a90f) with develop (9fdaa38)2

Open in CodSpeed

Footnotes

  1. 228 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.

  2. No successful run was found on mk/dbp-serde (72f2396) during the generation of this report, so develop (9fdaa38) was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

Comment thread vortex-file/src/writer.rs Outdated
.with_btrblocks_builder(
BtrBlocksCompressorBuilder::default()
.retain_allowed_encodings(&allowed_array_encodings),
.retain_allowed_encodings(&allowed_array_encodings)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this has to be serialised ids

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should somehow unify these 2 functions

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we can enable a scheme if at least one of associated serialized ids is enabled?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah actually I think this needs to be separate since there's a diff bw what ids MUST be allowed for a scheme to proc (all) vs what is optional (at least one).

FSST for example requires VarBin and FSST to be enabled. DBP requires at least one of v1 or v2.

@joseph-isaacs

Copy link
Copy Markdown
Contributor

Do you have an example of this being used?

@mhk197
mhk197 force-pushed the mk/compressor-serialized-ids branch from f5b732f to c27a6d6 Compare September 8, 2026 15:10

@mhk197 mhk197 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fn default() -> Self {
Self {
schemes: ALL_SCHEMES.to_vec(),
allowed_serialized_ids: None,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does None mean?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All allowed (e.g. editions not enforced)

#[derive(Debug, Clone)]
pub struct BtrBlocksCompressorBuilder {
schemes: Vec<&'static dyn Scheme>,
allowed_serialized_ids: Option<HashSet<ArrayId>>,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make this none optional

@mhk197
mhk197 removed this pull request from stack #9811 September 9, 2026 15:09
@mhk197
mhk197 force-pushed the mk/compressor-serialized-ids branch from 94ab744 to 8129237 Compare September 9, 2026 15:09
@mhk197
mhk197 changed the base branch from develop to mk/dbp-serde September 9, 2026 15:09
@mhk197
mhk197 added this pull request to stack #9813 September 9, 2026 15:09
@mhk197
mhk197 force-pushed the mk/compressor-serialized-ids branch from 8129237 to b28f0b6 Compare September 9, 2026 18:31
@mhk197
mhk197 force-pushed the mk/compressor-serialized-ids branch from b28f0b6 to 7accd56 Compare September 9, 2026 21:29
CascadingCompressor carries the snapshot of serialized IDs the writer may emit, filled by the file writer from the enabled editions through BtrBlocksCompressorBuilder::allow_serialized_ids. A scheme whose encoding has more than one wire format picks its compression mode from it with allows_serialized_id, the newest permitted one; without a restriction every ID is allowed. No scheme consults the set yet. This is the mechanism docs/specs/editions.md describes under compression with replacement encodings (#9779).

Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Scheme::produced_encodings now names the serialized IDs a scheme may write its output under, oldest first. BtrBlocksCompressorBuilder::allow_serialized_ids replaces retain_allowed_encodings: it keeps a scheme when at least one of those IDs is permitted and hands the set to the compressor, so the writer makes one call from the serialized IDs its editions permit instead of mapping them back to in-memory encodings, which could not tell two wire formats of one encoding apart.
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
The compressor seeds each root CompressorContext with its permitted serialized IDs and every descent inherits them, so a scheme asks compress_ctx.allows_serialized_id both while estimating and while compressing and picks the same mode in both. The per-compressor accessor goes; allowed_serialized_ids remains for inspection.

Signed-off-by: Matt Katz <mhkatz97@gmail.com>
…ed ids

Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Restore produced_encodings in feature-gated schemes and fix stale trait links. Document scheme ID resolution for Clippy.

Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
@joseph-isaacs
joseph-isaacs force-pushed the mk/compressor-serialized-ids branch from ece1fe2 to bf0a90f Compare September 10, 2026 15:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/chore A trivial change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants