remove nodes array

PSeitz · PSeitz · commit bd64dd23fef7 · 2024-05-17T19:59:31.000+08:00
diff --git a/common/src/lib.rs b/common/src/lib.rs
@@ -21,8 +21,7 @@ pub use json_path_writer::JsonPathWriter;
 pub use ownedbytes::{OwnedBytes, StableDeref};
 pub use serialize::{BinarySerializable, DeserializeFrom, FixedSize};
 pub use vint::{
-    read_u32_vint, read_u32_vint_no_advance, serialize_vint_u32,
-    vint_32_iterator as u32_vint_iterator, write_u32_vint, VInt, VIntU128,
+    read_u32_vint, read_u32_vint_no_advance, serialize_vint_u32, write_u32_vint, VInt, VIntU128,
 };
 pub use writer::{AntiCallToken, CountingWriter, TerminatingWrite};
 
diff --git a/common/src/serialize.rs b/common/src/serialize.rs
@@ -120,6 +120,23 @@ impl FixedSize for u32 {
     const SIZE_IN_BYTES: usize = 4;
 }
 
+impl BinarySerializable for [u8; 3] {
+    fn serialize<W: Write + ?Sized>(&self, writer: &mut W) -> io::Result<()> {
+        writer.write_u8(self[0])?;
+        writer.write_u8(self[1])?;
+        writer.write_u8(self[2])?;
+        Ok(())
+    }
+
+    fn deserialize<R: Read>(reader: &mut R) -> io::Result<[u8; 3]> {
+        Ok([reader.read_u8()?, reader.read_u8()?, reader.read_u8()?])
+    }
+}
+
+impl FixedSize for [u8; 3] {
+    const SIZE_IN_BYTES: usize = 3;
+}
+
 impl BinarySerializable for u16 {
     fn serialize<W: Write + ?Sized>(&self, writer: &mut W) -> io::Result<()> {
         writer.write_u16::<Endianness>(*self)
@@ -347,6 +364,12 @@ pub mod test {
         assert_eq!(serialize_test(String::from("富士さん見える。")), 1 + 3 * 8);
     }
 
+    #[test]
+    fn test_serialize_3bytes() {
+        let bytes: [u8; 3] = [1, 2, 3];
+        assert_eq!(serialize_test(bytes), 3);
+    }
+
     #[test]
     fn test_serialize_vec() {
         assert_eq!(serialize_test(Vec::<u8>::new()), 1);
diff --git a/common/src/vint.rs b/common/src/vint.rs
@@ -157,20 +157,6 @@ pub fn write_u32_vint<W: io::Write>(val: u32, writer: &mut W) -> io::Result<()>
     writer.write_all(data)
 }
 
-/// Iterates over all u32 encoded vints in the slice
-pub fn vint_32_iterator(data: &[u8]) -> impl Iterator<Item = u32> + '_ {
-    let mut slice = data;
-    std::iter::from_fn(move || {
-        if slice.is_empty() {
-            None
-        } else {
-            let (value, vlen) = read_u32_vint_no_advance(&slice);
-            slice = &slice[vlen..];
-            Some(value)
-        }
-    })
-}
-
 impl VInt {
     pub fn val(&self) -> u64 {
         self.0
@@ -236,8 +222,7 @@ impl BinarySerializable for VInt {
 #[cfg(test)]
 mod tests {
 
-    use super::{serialize_vint_u32, u32_vint_iterator, BinarySerializable, VInt};
-    use crate::write_u32_vint;
+    use super::{serialize_vint_u32, BinarySerializable, VInt};
 
     fn aux_test_vint(val: u64) {
         let mut v = [14u8; 10];
@@ -256,18 +241,6 @@ mod tests {
         assert_eq!(val, serdeser_val.0);
     }
 
-    #[test]
-    fn vint_iterator_test() {
-        let mut out = Vec::new();
-
-        let values: Vec<u32> = (0..100).collect();
-        for i in values.iter() {
-            write_u32_vint(*i, &mut out).unwrap();
-        }
-        let deser: Vec<u32> = u32_vint_iterator(&out).collect();
-        assert_eq!(values, deser);
-    }
-
     #[test]
     fn test_vint() {
         aux_test_vint(0);
diff --git a/src/schema/document/default_document.rs b/src/schema/document/default_document.rs
@@ -1,5 +1,5 @@
 use std::collections::{BTreeMap, HashMap, HashSet};
-use std::io;
+use std::io::{self, Read, Write};
 use std::net::Ipv6Addr;
 
 use columnar::MonotonicallyMappableToU128;
@@ -56,7 +56,9 @@ impl CompactDoc {
 
     /// Adding a facet to the document.
     pub fn add_facet<F>(&mut self, field: Field, path: F)
-    where Facet: From<F> {
+    where
+        Facet: From<F>,
+    {
         let facet = Facet::from(path);
         self.add_leaf_field_value(field, ReferenceValueLeaf::Facet(facet.encoded_str()));
     }
@@ -255,7 +257,9 @@ impl Eq for CompactDoc {}
 
 impl DocumentDeserialize for CompactDoc {
     fn deserialize<'de, D>(mut deserializer: D) -> Result<Self, DeserializeError>
-    where D: DocumentDeserializer<'de> {
+    where
+        D: DocumentDeserializer<'de>,
+    {
         let mut doc = CompactDoc::default();
         // TODO: Deserializing into OwnedValue is wasteful. The deserializer should be able to work
         // on slices and referenced data.
@@ -285,30 +289,14 @@ impl<'a> Value<'a> for CompactDocValue<'a> {
 #[derive(Debug, Clone)]
 /// A container to store tantivy Value
 struct CompactDocContainer {
-    /// A list of nodes, that are used to store the values of the document.
-    ///
-    /// ## Note on the design
-    /// We could use just a single vec `node_data` to store the nodes, but this would have a
-    /// downside. node_data has flexible sized elements compared to `nodes`. So when creating
-    /// a vec or document, we can reserve space for all od the direct child nodes upfront, and then
-    /// write into the nodes array without resizing. This is not possible with `node_data`. So
-    /// we use `nodes` to store the references to the actual data in `node_data`.
-    /// There would be 2 ways to use node_data instead of nodes:
-    /// - Instead of storing start and len for arrays and objects, we could a list of node pointers
-    /// to the bytes in node_data. This would require more memory.
-    /// - A two layer approach, where we when receiving an array/object, we would process the first
-    ///   level
-    /// subnodes and store the pos and len in and then handle then deeper levels. I don't like the
-    /// added complexity of this approach.
-    nodes: mediumvec::Vec32<ValueAddr>,
-    /// The `node_data` is a vec of bytes, where each value is serialized into bytes and stored. It
-    /// includes all the data of the document.
+    /// `node_data` is a vec of bytes, where each value is serialized into bytes and stored. It
+    /// includes all the data of the document and also metadata like where the nodes are located
+    /// in an object or array.
     node_data: mediumvec::Vec32<u8>,
 }
 impl Default for CompactDocContainer {
     fn default() -> Self {
         Self {
-            nodes: mediumvec::Vec32::with_capacity(4),
             // This should be at the lower end of the payload of a document
             // 512 byte is pretty small
             node_data: mediumvec::Vec32::with_capacity(512),
@@ -323,6 +311,21 @@ pub struct ValueAddr {
     type_id: ValueType,
     val: Addr, // this is the address, except for bool and null, which are inlined
 }
+impl BinarySerializable for ValueAddr {
+    fn serialize<W: Write + ?Sized>(&self, writer: &mut W) -> io::Result<()> {
+        (self.type_id as u8).serialize(writer)?;
+        self.val.0.serialize(writer)
+    }
+
+    fn deserialize<R: Read>(reader: &mut R) -> io::Result<Self> {
+        let type_id = ValueType::deserialize(reader)?;
+        let addr: [u8; 3] = <[u8; 3]>::deserialize(reader)?;
+        Ok(ValueAddr {
+            type_id,
+            val: Addr(addr),
+        })
+    }
+}
 impl std::fmt::Debug for ValueAddr {
     fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
         f.write_fmt(format_args!(
@@ -400,6 +403,26 @@ pub enum ValueType {
     Array = 12,
 }
 
+impl BinarySerializable for ValueType {
+    fn serialize<W: Write + ?Sized>(&self, writer: &mut W) -> io::Result<()> {
+        (*self as u8).serialize(writer)?;
+        Ok(())
+    }
+
+    fn deserialize<R: Read>(reader: &mut R) -> io::Result<Self> {
+        let num = u8::deserialize(reader)?;
+        let type_id = if (0..=12).contains(&num) {
+            unsafe { std::mem::transmute(num) }
+        } else {
+            return Err(io::Error::new(
+                io::ErrorKind::InvalidData,
+                format!("Invalid value type id: {}", num),
+            ));
+        };
+        Ok(type_id)
+    }
+}
+
 impl<'a, V: Value<'a>> From<&ReferenceValue<'a, V>> for ValueType {
     fn from(value: &ReferenceValue<'a, V>) -> Self {
         match value {
@@ -474,9 +497,10 @@ impl CompactDocContainer {
                 let mut positions = Vec::new();
                 for elem in elements {
                     let ref_elem = self.add_value(elem);
-                    let position = self.nodes.len() as u32;
+                    let position = self.node_data.len() as u32;
                     write_u32_vint(position, &mut positions).expect("in memory can't fail");
-                    self.nodes.push(ref_elem);
+                    write_into(&mut self.node_data, ref_elem);
+                    // self.nodes.push(ref_elem);
                 }
                 ValueAddr::new(type_id, write_bytes_into(&mut self.node_data, &positions))
             }
@@ -485,10 +509,10 @@ impl CompactDocContainer {
                 for (key, value) in entries {
                     let ref_key = self.add_value_leaf(ReferenceValueLeaf::Str(key));
                     let ref_value = self.add_value(value);
-                    let position = self.nodes.len() as u32;
+                    let position = self.node_data.len() as u32;
                     write_u32_vint(position, &mut positions).expect("in memory can't fail");
-                    self.nodes.push(ref_key);
-                    self.nodes.push(ref_value);
+                    write_into(&mut self.node_data, ref_key);
+                    write_into(&mut self.node_data, ref_value);
                 }
                 ValueAddr::new(type_id, write_bytes_into(&mut self.node_data, &positions))
             }
@@ -619,12 +643,15 @@ impl<'a> Iterator for CompactDocObjectIter<'a> {
     type Item = (&'a str, CompactDocValue<'a>);
 
     fn next(&mut self) -> Option<Self::Item> {
-        if self.positions_slice.len() > 0 {
+        if !self.positions_slice.is_empty() {
             let key_index = read_u32_vint(&mut self.positions_slice) as usize;
-            let key = self.container.extract_str(self.container.nodes[key_index]);
+            let position = &mut &self.container.node_data[key_index..];
+            let key_addr = ValueAddr::deserialize(position).ok()?;
+            let key = self.container.extract_str(key_addr);
+            let value = ValueAddr::deserialize(position).ok()?;
             let value = CompactDocValue {
                 container: self.container,
-                value: self.container.nodes[key_index + 1],
+                value,
             };
             return Some((key, value));
         }
@@ -653,11 +680,12 @@ impl<'a> Iterator for CompactDocArrayIter<'a> {
     type Item = CompactDocValue<'a>;
 
     fn next(&mut self) -> Option<Self::Item> {
-        if self.positions_slice.len() > 0 {
+        if !self.positions_slice.is_empty() {
             let key_index = read_u32_vint(&mut self.positions_slice) as usize;
+            let value = ValueAddr::deserialize(&mut &self.container.node_data[key_index..]).ok()?;
             let value = CompactDocValue {
                 container: self.container,
-                value: self.container.nodes[key_index],
+                value,
             };
             return Some(value);
         }
