diff --git a/fastfield_codecs/src/column.rs b/fastfield_codecs/src/column.rs index 02dbb804f..b20d39c97 100644 --- a/fastfield_codecs/src/column.rs +++ b/fastfield_codecs/src/column.rs @@ -53,43 +53,8 @@ pub trait Column: Send + Sync { } } -/// Concept of new Column API, which better accounts for null values. -pub trait ColumnV2 { - /// Return the value associated to the given idx. - /// - /// This accessor should return as fast as possible. - /// - /// # Panics - /// - /// May panic if `idx` is greater than the column length. - fn get_val(&self, idx: u64) -> Option; - - /// Returns the minimum value for this fast field. - /// - /// This min_value may not be exact. - /// For instance, the min value does not take in account of possible - /// deleted document. All values are however guaranteed to be higher than - /// `.min_value()`. - fn min_value(&self) -> T; - - /// Returns the maximum value for this fast field. - /// - /// This max_value may not be exact. - /// For instance, the max value does not take in account of possible - /// deleted document. All values are however guaranteed to be higher than - /// `.max_value()`. - fn max_value(&self) -> T; - - fn num_vals(&self) -> u64; - - /// Returns a iterator over the data - fn iter<'a>(&'a self) -> Box> + 'a> { - Box::new((0..self.num_vals()).map(|idx| self.get_val(idx))) - } -} - -/// Extend ColumnV2 Api -pub trait ColumnV2Ext: ColumnV2 { +/// Extend Column Api +pub trait ColumnExt: Column { /// Return the positions of values which are in the provided range. fn get_between_vals(&self, range: RangeInclusive) -> Vec; } @@ -152,44 +117,9 @@ impl<'a, T: Copy + PartialOrd + Send + Sync> Column for VecColumn<'a, T> { } } -impl<'a, T: Copy + PartialOrd> ColumnV2 for VecColumn<'a, T> { - fn get_val(&self, position: u64) -> Option { - Some(self.values[position as usize]) - } - - fn min_value(&self) -> T { - self.min_value - } - - fn max_value(&self) -> T { - self.max_value - } - - fn num_vals(&self) -> u64 { - self.values.len() as u64 - } -} - -impl<'a, T: Copy + PartialOrd> ColumnV2 for VecColumn<'a, Option> { - fn get_val(&self, position: u64) -> Option { - self.values[position as usize] - } - - fn min_value(&self) -> T { - self.min_value.unwrap() - } - - fn max_value(&self) -> T { - self.max_value.unwrap() - } - - fn num_vals(&self) -> u64 { - self.values.len() as u64 - } -} - impl<'a, T: Copy + Ord + Default, V> From<&'a V> for VecColumn<'a, T> -where V: AsRef<[T]> + ?Sized +where + V: AsRef<[T]> + ?Sized, { fn from(values: &'a V) -> Self { let values = values.as_ref(); @@ -287,7 +217,8 @@ where pub struct IterColumn(T); impl From for IterColumn -where T: Iterator + Clone + ExactSizeIterator +where + T: Iterator + Clone + ExactSizeIterator, { fn from(iter: T) -> Self { IterColumn(iter) diff --git a/fastfield_codecs/src/compact_space/mod.rs b/fastfield_codecs/src/compact_space/mod.rs index 7428ca8e7..2c13e508a 100644 --- a/fastfield_codecs/src/compact_space/mod.rs +++ b/fastfield_codecs/src/compact_space/mod.rs @@ -22,8 +22,8 @@ use common::{BinarySerializable, CountingWriter, VInt, VIntU128}; use ownedbytes::OwnedBytes; use tantivy_bitpacker::{self, BitPacker, BitUnpacker}; -use crate::column::{ColumnV2, ColumnV2Ext}; use crate::compact_space::build_compact_space::get_compact_space; +use crate::{column::ColumnExt, Column}; mod blank_range; mod build_compact_space; @@ -36,8 +36,6 @@ pub fn ip_to_u128(ip_addr: IpAddr) -> u128 { u128::from_be_bytes(ip_addr_v6.octets()) } -const NULL_VALUE_COMPACT_SPACE: u64 = 0; - /// The cost per blank is quite hard actually, since blanks are delta encoded, the actual cost of /// blanks depends on the number of blanks. /// @@ -182,9 +180,9 @@ pub struct IPCodecParams { impl CompactSpaceCompressor { /// Taking the vals as Vec may cost a lot of memory. It is used to sort the vals. - pub fn train_from(column: impl ColumnV2) -> Self { + pub fn train_from(column: impl Column) -> Self { let mut values_sorted = BTreeSet::new(); - values_sorted.extend(column.iter().flatten()); + values_sorted.extend(column.iter()); let total_num_values = column.num_vals(); let compact_space = @@ -227,7 +225,7 @@ impl CompactSpaceCompressor { Ok(()) } - pub fn compress(self, vals: impl Iterator>) -> io::Result> { + pub fn compress(self, vals: impl Iterator) -> io::Result> { let mut output = vec![]; self.compress_into(vals, &mut output)?; Ok(output) @@ -235,24 +233,21 @@ impl CompactSpaceCompressor { pub fn compress_into( self, - vals: impl Iterator>, + vals: impl Iterator, write: &mut impl Write, ) -> io::Result<()> { let mut bitpacker = BitPacker::default(); for val in vals { - let compact = if let Some(val) = val { - self.params - .compact_space - .u128_to_compact(val) - .map_err(|_| { - io::Error::new( - io::ErrorKind::InvalidData, - "Could not convert value to compact_space. This is a bug.", - ) - })? - } else { - NULL_VALUE_COMPACT_SPACE - }; + let compact = self + .params + .compact_space + .u128_to_compact(val) + .map_err(|_| { + io::Error::new( + io::ErrorKind::InvalidData, + "Could not convert value to compact_space. This is a bug.", + ) + })?; bitpacker.write(compact, self.params.num_bits, write)?; } bitpacker.close(write)?; @@ -302,9 +297,9 @@ impl BinarySerializable for IPCodecParams { } } -impl ColumnV2 for CompactSpaceDecompressor { +impl Column for CompactSpaceDecompressor { #[inline] - fn get_val(&self, doc: u64) -> Option { + fn get_val(&self, doc: u64) -> u128 { self.get(doc) } @@ -321,12 +316,12 @@ impl ColumnV2 for CompactSpaceDecompressor { } #[inline] - fn iter<'a>(&'a self) -> Box> + 'a> { + fn iter<'a>(&'a self) -> Box + 'a> { Box::new(self.iter()) } } -impl ColumnV2Ext for CompactSpaceDecompressor { +impl ColumnExt for CompactSpaceDecompressor { fn get_between_vals(&self, range: RangeInclusive) -> Vec { self.get_range(range) } @@ -440,26 +435,17 @@ impl CompactSpaceDecompressor { } #[inline] - fn iter(&self) -> impl Iterator> + '_ { + fn iter(&self) -> impl Iterator + '_ { // TODO: Performance. It would be better to iterate on the ranges and check existence via // the bit_unpacker. - self.iter_compact().map(|compact| { - if compact == NULL_VALUE_COMPACT_SPACE { - None - } else { - Some(self.compact_to_u128(compact)) - } - }) + self.iter_compact() + .map(|compact| self.compact_to_u128(compact)) } #[inline] - pub fn get(&self, idx: u64) -> Option { + pub fn get(&self, idx: u64) -> u128 { let compact = self.params.bit_unpacker.get(idx, &self.data); - if compact == NULL_VALUE_COMPACT_SPACE { - None - } else { - Some(self.compact_to_u128(compact)) - } + self.compact_to_u128(compact) } pub fn min_value(&self) -> u128 { @@ -520,32 +506,30 @@ mod tests { assert_eq!(amplitude, 2); } - fn test_all(data: OwnedBytes, expected: &[Option]) { + fn test_all(data: OwnedBytes, expected: &[u128]) { let decompressor = CompactSpaceDecompressor::open(data).unwrap(); for (idx, expected_val) in expected.iter().cloned().enumerate() { let val = decompressor.get(idx as u64); assert_eq!(val, expected_val); - if let Some(expected_val) = expected_val { - let test_range = |range: RangeInclusive| { - let expected_positions = expected - .iter() - .positions(|val| val.map(|val| range.contains(&val)).unwrap_or(false)) - .map(|pos| pos as u64) - .collect::>(); - let positions = decompressor.get_range(range); - assert_eq!(positions, expected_positions); - }; + let test_range = |range: RangeInclusive| { + let expected_positions = expected + .iter() + .positions(|val| range.contains(val)) + .map(|pos| pos as u64) + .collect::>(); + let positions = decompressor.get_range(range); + assert_eq!(positions, expected_positions); + }; - test_range(expected_val.saturating_sub(1)..=expected_val); - test_range(expected_val..=expected_val); - test_range(expected_val..=expected_val.saturating_add(1)); - test_range(expected_val.saturating_sub(1)..=expected_val.saturating_add(1)); - } + test_range(expected_val.saturating_sub(1)..=expected_val); + test_range(expected_val..=expected_val); + test_range(expected_val..=expected_val.saturating_add(1)); + test_range(expected_val.saturating_sub(1)..=expected_val.saturating_add(1)); } } - fn test_aux_vals_opt(u128_vals: &[Option]) -> OwnedBytes { + fn test_aux_vals(u128_vals: &[u128]) -> OwnedBytes { let compressor = CompactSpaceCompressor::train_from(VecColumn::from(u128_vals)); let data = compressor.compress(u128_vals.iter().cloned()).unwrap(); let data = OwnedBytes::new(data); @@ -553,10 +537,6 @@ mod tests { data } - fn test_aux_vals(u128_vals: &[u128]) -> OwnedBytes { - test_aux_vals_opt(&u128_vals.iter().cloned().map(Some).collect::>()) - } - #[test] fn test_range_1() { let vals = &[ @@ -622,30 +602,6 @@ mod tests { assert_eq!(positions, vec![0]); } - #[test] - fn test_null() { - let vals = vec![None, Some(2u128)]; - let compressor = CompactSpaceCompressor::train_from(VecColumn::from(&vals)); - let data = compressor.compress(vals.iter().cloned()).unwrap(); - let decomp = CompactSpaceDecompressor::open(OwnedBytes::new(data)).unwrap(); - let positions = decomp.get_range(0..=1); - assert_eq!(positions, vec![]); - let positions = decomp.get_range(2..=2); - assert_eq!(positions, vec![1]); - - let positions = decomp.get_range(2..=3); - assert_eq!(positions, vec![1]); - - let positions = decomp.get_range(1..=3); - assert_eq!(positions, vec![1]); - - let positions = decomp.get_range(2..=3); - assert_eq!(positions, vec![1]); - - let positions = decomp.get_range(3..=3); - assert_eq!(positions, vec![]); - } - #[test] fn test_range_3() { let vals = &[ @@ -664,7 +620,7 @@ mod tests { 5_000_000_000, ]; let compressor = CompactSpaceCompressor::train_from(VecColumn::from(vals)); - let data = compressor.compress(vals.iter().cloned().map(Some)).unwrap(); + let data = compressor.compress(vals.iter().cloned()).unwrap(); let decomp = CompactSpaceDecompressor::open(OwnedBytes::new(data)).unwrap(); assert_eq!(decomp.get_range(199..=200), vec![0]); diff --git a/fastfield_codecs/src/lib.rs b/fastfield_codecs/src/lib.rs index d2adf6500..ab3842cb4 100644 --- a/fastfield_codecs/src/lib.rs +++ b/fastfield_codecs/src/lib.rs @@ -338,7 +338,7 @@ mod bench { use std::iter; use std::sync::Arc; - use column::ColumnV2Ext; + use column::ColumnExt; use rand::prelude::*; use test::{self, Bencher}; @@ -386,33 +386,26 @@ mod bench { }); } - fn get_u128_column_permutation() -> Arc> { + fn get_u128_column_permutation() -> Arc> { let permutation = generate_permutation(); - let permutation = permutation - .iter() - .map(|el| *el as u128) - .map(Some) - .collect::>(); + let permutation = permutation.iter().map(|el| *el as u128).collect::>(); get_u128_column(&permutation) } - fn get_data_50percent_item() -> (u128, u128, Vec>) { + fn get_data_50percent_item() -> (u128, u128, Vec) { let mut permutation = generate_permutation(); let major_item = permutation[0]; let minor_item = permutation[1]; permutation.extend(iter::repeat(major_item).take(permutation.len())); permutation.shuffle(&mut StdRng::from_seed([1u8; 32])); - let permutation = permutation - .iter() - .map(|el| Some(*el as u128)) - .collect::>(); + let permutation = permutation.iter().map(|el| *el as u128).collect::>(); (major_item as u128, minor_item as u128, permutation) } - fn get_u128_column(data: &[Option]) -> Arc> { + fn get_u128_column(data: &[u128]) -> Arc> { let compressor = CompactSpaceCompressor::train_from(VecColumn::from(&data)); let data = compressor.compress(data.iter().cloned()).unwrap(); let data = OwnedBytes::new(data); - let column: Arc> = + let column: Arc> = Arc::new(CompactSpaceDecompressor::open(data).unwrap()); column } @@ -448,7 +441,7 @@ mod bench { b.iter(|| { let mut a = 0u128; for _ in 0..column.num_vals() { - a = column.get_val(a as u64).unwrap(); + a = column.get_val(a as u64); } a }); @@ -462,7 +455,7 @@ mod bench { let n = column.num_vals(); let mut a = 0u128; for i in (0..n / 5).map(|val| val * 5) { - a += column.get_val(i as u64).unwrap(); + a += column.get_val(i as u64); } a }); diff --git a/fastfield_codecs/src/main.rs b/fastfield_codecs/src/main.rs index ef39df0b9..3ec4cf15d 100644 --- a/fastfield_codecs/src/main.rs +++ b/fastfield_codecs/src/main.rs @@ -95,7 +95,7 @@ fn bench_ip() { for dataset in dataset.chunks(50_000) { let compressor = CompactSpaceCompressor::train_from(VecColumn::from(dataset)); compressor - .compress_into(dataset.iter().cloned().map(Some), &mut data) + .compress_into(dataset.iter().cloned(), &mut data) .unwrap(); } let compression = data.len() as f64 / (dataset.len() * 16) as f64; @@ -107,9 +107,7 @@ fn bench_ip() { } let compressor = CompactSpaceCompressor::train_from(VecColumn::from(&dataset)); - let data = compressor - .compress(dataset.iter().cloned().map(Some)) - .unwrap(); + let data = compressor.compress(dataset.iter().cloned()).unwrap(); let compression = data.len() as f64 / (dataset.len() * 16) as f64; println!("Compression {:.2}", compression);