Merge pull request #1509 from quickwit-oss/refact-fast-field

refactor, fix api

Author: PSeitz

fastfield_codecs/benches/bench.rs

@@ -6,9 +6,6 @@ extern crate test;
 mod tests {
     use std::sync::Arc;
 
-    use fastfield_codecs::bitpacked::BitpackedCodec;
-    use fastfield_codecs::blockwise_linear::BlockwiseLinearCodec;
-    use fastfield_codecs::linear::LinearCodec;
     use fastfield_codecs::*;
 
     fn get_data() -> Vec<u64> {

fastfield_codecs/src/column.rs

@@ -176,7 +176,7 @@ where
     T: Copy + Ord + Default,
 {
     fn min_max(&self) -> (T, T) {
-        if let Some((min, max)) = self.min_max_cache.lock().unwrap().clone() {
+        if let Some((min, max)) = *self.min_max_cache.lock().unwrap() {
             return (min, max);
         }
         let (min, max) =

fastfield_codecs/src/lib.rs

@@ -9,21 +9,28 @@ extern crate test;
 
 use std::io;
 use std::io::Write;
+use std::sync::Arc;
 
 use common::BinarySerializable;
 use ownedbytes::OwnedBytes;
+use serialize::Header;
 
 mod bitpacked;
 mod blockwise_linear;
 pub(crate) mod line;
 mod linear;
+mod monotonic_mapping;
 
 mod column;
 mod gcd;
 mod serialize;
 
+pub use self::bitpacked::BitpackedCodec;
+pub use self::blockwise_linear::BlockwiseLinearCodec;
 pub use self::column::{monotonic_map_column, Column, VecColumn};
-pub use self::serialize::{estimate, open, serialize, serialize_and_load, NormalizedHeader};
+pub use self::linear::LinearCodec;
+pub use self::monotonic_mapping::MonotonicallyMappableToU64;
+pub use self::serialize::{estimate, serialize, serialize_and_load, NormalizedHeader};
 
 #[derive(PartialEq, Eq, PartialOrd, Ord, Debug, Clone, Copy)]
 #[repr(u8)]
@@ -61,70 +68,39 @@ impl FastFieldCodecType {
     }
 }
 
-pub trait MonotonicallyMappableToU64: 'static + PartialOrd + Copy {
-    /// Converts a value to u64.
-    ///
-    /// Internally all fast field values are encoded as u64.
-    fn to_u64(self) -> u64;
-
-    /// Converts a value from u64
-    ///
-    /// Internally all fast field values are encoded as u64.
-    /// **Note: To be used for converting encoded Term, Posting values.**
-    fn from_u64(val: u64) -> Self;
-}
-
-impl MonotonicallyMappableToU64 for u64 {
-    fn to_u64(self) -> u64 {
-        self
-    }
-
-    fn from_u64(val: u64) -> Self {
-        val
-    }
-}
-
-impl MonotonicallyMappableToU64 for i64 {
-    #[inline(always)]
-    fn to_u64(self) -> u64 {
-        common::i64_to_u64(self)
-    }
-
-    #[inline(always)]
-    fn from_u64(val: u64) -> Self {
-        common::u64_to_i64(val)
-    }
-}
-
-impl MonotonicallyMappableToU64 for bool {
-    #[inline(always)]
-    fn to_u64(self) -> u64 {
-        if self {
-            1
-        } else {
-            0
+/// Returns the correct codec reader wrapped in the `Arc` for the data.
+pub fn open<T: MonotonicallyMappableToU64>(
+    mut bytes: OwnedBytes,
+) -> io::Result<Arc<dyn Column<T>>> {
+    let header = Header::deserialize(&mut bytes)?;
+    match header.codec_type {
+        FastFieldCodecType::Bitpacked => open_specific_codec::<BitpackedCodec, _>(bytes, &header),
+        FastFieldCodecType::Linear => open_specific_codec::<LinearCodec, _>(bytes, &header),
+        FastFieldCodecType::BlockwiseLinear => {
+            open_specific_codec::<BlockwiseLinearCodec, _>(bytes, &header)
         }
     }
-
-    #[inline(always)]
-    fn from_u64(val: u64) -> Self {
-        val > 0
-    }
 }
 
-impl MonotonicallyMappableToU64 for f64 {
-    fn to_u64(self) -> u64 {
-        common::f64_to_u64(self)
-    }
-
-    fn from_u64(val: u64) -> Self {
-        common::u64_to_f64(val)
+fn open_specific_codec<C: FastFieldCodec, Item: MonotonicallyMappableToU64>(
+    bytes: OwnedBytes,
+    header: &Header,
+) -> io::Result<Arc<dyn Column<Item>>> {
+    let normalized_header = header.normalized();
+    let reader = C::open_from_bytes(bytes, normalized_header)?;
+    let min_value = header.min_value;
+    if let Some(gcd) = header.gcd {
+        let monotonic_mapping = move |val: u64| Item::from_u64(min_value + val * gcd.get());
+        Ok(Arc::new(monotonic_map_column(reader, monotonic_mapping)))
+    } else {
+        let monotonic_mapping = move |val: u64| Item::from_u64(min_value + val);
+        Ok(Arc::new(monotonic_map_column(reader, monotonic_mapping)))
     }
 }
 
 /// The FastFieldSerializerEstimate trait is required on all variants
 /// of fast field compressions, to decide which one to choose.
-trait FastFieldCodec: 'static {
+pub trait FastFieldCodec: 'static {
     /// A codex needs to provide a unique name and id, which is
     /// used for debugging and de/serialization.
     const CODEC_TYPE: FastFieldCodecType;

fastfield_codecs/src/monotonic_mapping.rs

@@ -0,0 +1,60 @@
+pub trait MonotonicallyMappableToU64: 'static + PartialOrd + Copy {
+    /// Converts a value to u64.
+    ///
+    /// Internally all fast field values are encoded as u64.
+    fn to_u64(self) -> u64;
+
+    /// Converts a value from u64
+    ///
+    /// Internally all fast field values are encoded as u64.
+    /// **Note: To be used for converting encoded Term, Posting values.**
+    fn from_u64(val: u64) -> Self;
+}
+
+impl MonotonicallyMappableToU64 for u64 {
+    fn to_u64(self) -> u64 {
+        self
+    }
+
+    fn from_u64(val: u64) -> Self {
+        val
+    }
+}
+
+impl MonotonicallyMappableToU64 for i64 {
+    #[inline(always)]
+    fn to_u64(self) -> u64 {
+        common::i64_to_u64(self)
+    }
+
+    #[inline(always)]
+    fn from_u64(val: u64) -> Self {
+        common::u64_to_i64(val)
+    }
+}
+
+impl MonotonicallyMappableToU64 for bool {
+    #[inline(always)]
+    fn to_u64(self) -> u64 {
+        if self {
+            1
+        } else {
+            0
+        }
+    }
+
+    #[inline(always)]
+    fn from_u64(val: u64) -> Self {
+        val > 0
+    }
+}
+
+impl MonotonicallyMappableToU64 for f64 {
+    fn to_u64(self) -> u64 {
+        common::f64_to_u64(self)
+    }
+
+    fn from_u64(val: u64) -> Self {
+        common::u64_to_f64(val)
+    }
+}

fastfield_codecs/src/serialize.rs

@@ -47,11 +47,11 @@ pub struct NormalizedHeader {
 
 #[derive(Debug, Copy, Clone)]
 pub(crate) struct Header {
-    num_vals: u64,
-    min_value: u64,
-    max_value: u64,
-    gcd: Option<NonZeroU64>,
-    codec_type: FastFieldCodecType,
+    pub num_vals: u64,
+    pub min_value: u64,
+    pub max_value: u64,
+    pub gcd: Option<NonZeroU64>,
+    pub codec_type: FastFieldCodecType,
 }
 
 impl Header {
@@ -124,36 +124,6 @@ impl BinarySerializable for Header {
     }
 }
 
-/// Returns correct the reader wrapped in the `DynamicFastFieldReader` enum for the data.
-pub fn open<T: MonotonicallyMappableToU64>(
-    mut bytes: OwnedBytes,
-) -> io::Result<Arc<dyn Column<T>>> {
-    let header = Header::deserialize(&mut bytes)?;
-    match header.codec_type {
-        FastFieldCodecType::Bitpacked => open_specific_codec::<BitpackedCodec, _>(bytes, &header),
-        FastFieldCodecType::Linear => open_specific_codec::<LinearCodec, _>(bytes, &header),
-        FastFieldCodecType::BlockwiseLinear => {
-            open_specific_codec::<BlockwiseLinearCodec, _>(bytes, &header)
-        }
-    }
-}
-
-fn open_specific_codec<C: FastFieldCodec, Item: MonotonicallyMappableToU64>(
-    bytes: OwnedBytes,
-    header: &Header,
-) -> io::Result<Arc<dyn Column<Item>>> {
-    let normalized_header = header.normalized();
-    let reader = C::open_from_bytes(bytes, normalized_header)?;
-    let min_value = header.min_value;
-    if let Some(gcd) = header.gcd {
-        let monotonic_mapping = move |val: u64| Item::from_u64(min_value + val * gcd.get());
-        Ok(Arc::new(monotonic_map_column(reader, monotonic_mapping)))
-    } else {
-        let monotonic_mapping = move |val: u64| Item::from_u64(min_value + val);
-        Ok(Arc::new(monotonic_map_column(reader, monotonic_mapping)))
-    }
-}
-
 pub fn estimate<T: MonotonicallyMappableToU64>(
     typed_column: impl Column<T>,
     codec_type: FastFieldCodecType,
@@ -217,8 +187,7 @@ fn detect_codec(
     // removing nan values for codecs with broken calculations, and max values which disables
     // codecs
     estimations.retain(|estimation| !estimation.0.is_nan() && estimation.0 != f32::MAX);
-    estimations
-        .sort_by(|(score_left, _), (score_right, _)| score_left.partial_cmp(&score_right).unwrap());
+    estimations.sort_by(|(score_left, _), (score_right, _)| score_left.total_cmp(score_right));
     Some(estimations.first()?.1)
 }
 

src/fastfield/bytes/writer.rs

@@ -1,4 +1,4 @@
-use std::io;
+use std::io::{self, Write};
 
 use fastfield_codecs::VecColumn;
 
@@ -112,7 +112,6 @@ impl BytesFastFieldWriter {
         doc_id_map: Option<&DocIdMapping>,
     ) -> io::Result<()> {
         // writing the offset index
-        // TODO FIXME No need to double the memory.
         {
             self.doc_index.push(self.vals.len() as u64);
             let col = VecColumn::from(&self.doc_index[..]);
@@ -128,7 +127,7 @@ impl BytesFastFieldWriter {
             }
         }
         // writing the values themselves
-        let mut value_serializer = serializer.new_bytes_fast_field_with_idx(self.field, 1);
+        let mut value_serializer = serializer.new_bytes_fast_field(self.field);
         // the else could be removed, but this is faster (difference not benchmarked)
         if let Some(doc_id_map) = doc_id_map {
             for vals in self.get_ordered_values(Some(doc_id_map)) {