Skip to content

Commit 1752d46

Browse files
committed
add predecessor schemes and make produced_encodings refer to serialized ids
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
1 parent dafbfa4 commit 1752d46

16 files changed

Lines changed: 686 additions & 405 deletions

File tree

Lines changed: 32 additions & 185 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use crate::schemes::integer;
1818
use crate::schemes::string;
1919
use crate::schemes::temporal;
2020

21-
/// All available compression schemes.
21+
/// The newest versions of all available compression schemes.
2222
///
2323
/// This list is order-sensitive: the builder preserves this order when constructing
2424
/// the final scheme list, so that tie-breaking is deterministic.
@@ -117,7 +117,7 @@ impl BtrBlocksCompressorBuilder {
117117
/// Adds an external compression scheme not in [`ALL_SCHEMES`].
118118
///
119119
/// This allows encoding crates outside of `vortex-btrblocks` to register their own schemes
120-
/// with the compressor.
120+
/// with the compressor. Register only the newest version of a scheme.
121121
///
122122
/// # Panics
123123
///
@@ -201,32 +201,53 @@ impl BtrBlocksCompressorBuilder {
201201
}
202202

203203
/// Removes the specified compression schemes by their [`SchemeId`].
204+
///
205+
/// An ID anywhere in a registered predecessor chain removes the entire chain.
206+
///
207+
/// # Panics
208+
///
209+
/// Panics if a traversed predecessor chain contains a cycle.
204210
pub fn exclude_schemes(mut self, ids: impl IntoIterator<Item = SchemeId>) -> Self {
205211
let ids: HashSet<_> = ids.into_iter().collect();
206-
self.schemes.retain(|s| !ids.contains(&s.id()));
212+
self.schemes.retain(|scheme| {
213+
let mut seen = HashSet::new();
214+
let mut candidate = Some(*scheme);
215+
while let Some(version) = candidate {
216+
assert!(
217+
seen.insert(version.id()),
218+
"cycle in scheme predecessor chain"
219+
);
220+
if ids.contains(&version.id()) {
221+
return false;
222+
}
223+
candidate = version.predecessor();
224+
}
225+
true
226+
});
207227
self
208228
}
209229

210230
/// Restricts compression to the serialized IDs in `allowed`, intersecting with any earlier
211231
/// call.
212232
///
213-
/// A scheme stays when at least one of its [produced IDs](Scheme::produced_encodings) is
214-
/// permitted, and the compressor is handed the set so a scheme whose encoding has several
215-
/// wire formats picks its compression mode from it: the newest permitted one.
216-
///
217-
/// The file writer passes the serialized IDs its enabled editions permit.
233+
/// At build time, each scheme is replaced by the newest version in its predecessor chain
234+
/// whose [`required_serialized_ids`](Scheme::required_serialized_ids) are all permitted.
235+
/// Schemes with no eligible version are removed. This also applies to schemes added after
236+
/// this call. The file writer passes the serialized IDs its enabled editions permit.
218237
pub fn allow_serialized_ids(mut self, allowed: &HashSet<ArrayId>) -> Self {
219238
let allowed: HashSet<ArrayId> = match self.allowed_serialized_ids.take() {
220239
Some(existing) => existing.intersection(allowed).copied().collect(),
221240
None => allowed.clone(),
222241
};
223-
self.schemes
224-
.retain(|s| s.produced_encodings().iter().any(|id| allowed.contains(id)));
225242
self.allowed_serialized_ids = Some(allowed);
226243
self
227244
}
228245

229246
/// Builds the configured [`BtrBlocksCompressor`].
247+
///
248+
/// # Panics
249+
///
250+
/// Panics if predecessor chains contain a cycle or share a scheme ID.
230251
pub fn build(self) -> BtrBlocksCompressor {
231252
let compressor = CascadingCompressor::new(self.schemes);
232253
BtrBlocksCompressor(match self.allowed_serialized_ids {
@@ -237,178 +258,4 @@ impl BtrBlocksCompressorBuilder {
237258
}
238259

239260
#[cfg(test)]
240-
mod tests {
241-
use vortex_array::ArrayRef;
242-
use vortex_array::Canonical;
243-
use vortex_array::ExecutionCtx;
244-
use vortex_array::VTable;
245-
use vortex_array::arrays::Bool;
246-
use vortex_array::arrays::Primitive;
247-
use vortex_compressor::scheme::CompressionEstimate;
248-
use vortex_compressor::scheme::EstimateVerdict;
249-
use vortex_error::VortexResult;
250-
use vortex_fastlanes::FoR;
251-
252-
use super::*;
253-
use crate::ArrayAndStats;
254-
use crate::CompressorContext;
255-
256-
#[test]
257-
fn empty_starts_with_no_schemes() {
258-
let builder = BtrBlocksCompressorBuilder::empty();
259-
assert!(builder.schemes.is_empty());
260-
}
261-
262-
#[test]
263-
fn default_includes_all_schemes() {
264-
let builder = BtrBlocksCompressorBuilder::default();
265-
assert_eq!(builder.schemes.len(), ALL_SCHEMES.len());
266-
}
267-
268-
#[test]
269-
fn allow_serialized_ids_filters_schemes() {
270-
let allowed: HashSet<ArrayId> = [FoR.id()].into_iter().collect();
271-
let builder = BtrBlocksCompressorBuilder::default().allow_serialized_ids(&allowed);
272-
assert_eq!(builder.schemes.len(), 1);
273-
assert_eq!(builder.schemes[0].id(), integer::FoRScheme.id());
274-
275-
let none = BtrBlocksCompressorBuilder::default().allow_serialized_ids(&HashSet::new());
276-
assert!(none.schemes.is_empty());
277-
}
278-
279-
#[test]
280-
fn allowing_all_declared_outputs_keeps_every_scheme() {
281-
let allowed: HashSet<ArrayId> = ALL_SCHEMES
282-
.iter()
283-
.flat_map(|scheme| scheme.produced_encodings())
284-
.collect();
285-
let builder = BtrBlocksCompressorBuilder::default().allow_serialized_ids(&allowed);
286-
assert_eq!(builder.schemes.len(), ALL_SCHEMES.len());
287-
}
288-
289-
/// Stands in for a scheme whose encoding has two wire formats.
290-
#[derive(Debug)]
291-
struct TwoFormatScheme;
292-
293-
impl Scheme for TwoFormatScheme {
294-
fn scheme_name(&self) -> &'static str {
295-
"test.two_formats"
296-
}
297-
298-
fn matches(&self, _canonical: &Canonical) -> bool {
299-
false
300-
}
301-
302-
fn produced_encodings(&self) -> Vec<ArrayId> {
303-
vec![FoR.id(), Bool.id()]
304-
}
305-
306-
fn expected_compression_ratio(
307-
&self,
308-
_data: &ArrayAndStats,
309-
_compress_ctx: CompressorContext,
310-
_exec_ctx: &mut ExecutionCtx,
311-
) -> CompressionEstimate {
312-
CompressionEstimate::Verdict(EstimateVerdict::Skip)
313-
}
314-
315-
fn compress(
316-
&self,
317-
_compressor: &CascadingCompressor,
318-
_data: &ArrayAndStats,
319-
_compress_ctx: CompressorContext,
320-
_exec_ctx: &mut ExecutionCtx,
321-
) -> VortexResult<ArrayRef> {
322-
unreachable!("test helper never matches")
323-
}
324-
}
325-
326-
/// A scheme with several wire formats stays while any of them is permitted; which one it
327-
/// produces is decided when compressing.
328-
#[test]
329-
fn any_permitted_format_keeps_the_scheme() {
330-
static TWO_FORMATS: TwoFormatScheme = TwoFormatScheme;
331-
332-
let newer_only = BtrBlocksCompressorBuilder::empty()
333-
.with_new_scheme(&TWO_FORMATS)
334-
.allow_serialized_ids(&HashSet::from([Bool.id()]));
335-
assert_eq!(newer_only.schemes.len(), 1);
336-
337-
let neither = BtrBlocksCompressorBuilder::empty()
338-
.with_new_scheme(&TWO_FORMATS)
339-
.allow_serialized_ids(&HashSet::from([Primitive.id()]));
340-
assert!(neither.schemes.is_empty());
341-
}
342-
343-
#[test]
344-
fn cuda_compatible_excludes_alprd() {
345-
let builder = BtrBlocksCompressorBuilder::default().only_cuda_compatible();
346-
assert!(
347-
!builder
348-
.schemes
349-
.iter()
350-
.any(|s| s.id() == float::ALPRDScheme.id())
351-
);
352-
}
353-
354-
/// `vortex.sparse` has no CUDA decode kernel, so no sparse scheme may survive this preset.
355-
#[test]
356-
fn cuda_compatible_excludes_every_sparse_scheme() {
357-
let builder = BtrBlocksCompressorBuilder::default().only_cuda_compatible();
358-
for excluded in [
359-
integer::SparseScheme.id(),
360-
float::NullDominatedSparseScheme.id(),
361-
string::NullDominatedSparseScheme.id(),
362-
] {
363-
assert!(
364-
!builder.schemes.iter().any(|s| s.id() == excluded),
365-
"{excluded} should be excluded"
366-
);
367-
}
368-
}
369-
370-
/// Every serialized ID is allowed until the writer narrows the set to its editions.
371-
#[test]
372-
fn allowed_serialized_ids_reach_the_compressor() {
373-
let default = BtrBlocksCompressorBuilder::default().build();
374-
assert!(default.0.allowed_serialized_ids().is_none());
375-
376-
let narrowed = BtrBlocksCompressorBuilder::default()
377-
.allow_serialized_ids(&HashSet::from([FoR.id()]))
378-
.build();
379-
assert_eq!(
380-
narrowed.0.allowed_serialized_ids(),
381-
Some(&HashSet::from([FoR.id()]))
382-
);
383-
}
384-
385-
#[test]
386-
fn cuda_compatible_uses_fsst_for_strings() {
387-
let builder = BtrBlocksCompressorBuilder::default().only_cuda_compatible();
388-
assert!(
389-
builder
390-
.schemes
391-
.iter()
392-
.any(|scheme| scheme.id() == string::FSSTScheme.id())
393-
);
394-
#[cfg(feature = "zstd")]
395-
assert!(
396-
!builder
397-
.schemes
398-
.iter()
399-
.any(|scheme| scheme.id() == string::ZstdScheme.id())
400-
);
401-
}
402-
403-
#[test]
404-
#[cfg(feature = "pco")]
405-
fn cuda_compatible_excludes_pco() {
406-
let builder = BtrBlocksCompressorBuilder::default()
407-
.with_new_scheme(&integer::PcoScheme)
408-
.with_new_scheme(&float::PcoScheme)
409-
.only_cuda_compatible();
410-
for scheme in [integer::PcoScheme.id(), float::PcoScheme.id()] {
411-
assert!(!builder.schemes.iter().any(|s| s.id() == scheme));
412-
}
413-
}
414-
}
261+
mod tests;

0 commit comments

Comments
 (0)