Merge pull request #117 from fjall-rs/perf/skip-range-key-cmps

Skip superfluous range bound key comparisons

Author: marvin-j97

Cargo.toml

@@ -37,7 +37,7 @@ quick_cache = { version = "0.6.5", default-features = false, features = [] }
 rustc-hash = "2.0.0"
 self_cell = "1.0.4"
 tempfile = "3.12.0"
-value-log = { version = "1.6.0", default-features = false, features = [] }
+value-log = { version = "1.7.2", default-features = false, features = [] }
 varint-rs = "2.2.0"
 xxhash-rust = { version = "0.8.12", features = ["xxh3"] }
 

benches/tli.rs

@@ -1,6 +1,6 @@
 use criterion::{criterion_group, criterion_main, Criterion};
 use lsm_tree::segment::{
-    block_index::KeyedBlockIndex, value_block::BlockOffset, value_block::CachePolicy,
+    block::offset::BlockOffset, block_index::KeyedBlockIndex, value_block::CachePolicy,
 };
 
 fn tli_find_item(c: &mut Criterion) {

src/abstract.rs

@@ -106,14 +106,16 @@ pub trait AbstractTree {
     /// Returns `None` if the level does not exist (if idx >= 7).
     fn level_segment_count(&self, idx: usize) -> Option<usize>;
 
+    /// Returns the amount of disjoint runs in L0.
+    ///
+    /// Can be used to determine whether to write stall.
+    fn l0_run_count(&self) -> usize;
+
     /// Returns the amount of blob files currently in the tree.
     fn blob_file_count(&self) -> usize {
         0
     }
 
-    /// Returns `true` if the first level is disjoint.
-    fn is_first_level_disjoint(&self) -> bool;
-
     /// Approximates the amount of items in the tree.
     fn approximate_len(&self) -> usize;
 

src/blob_tree/mod.rs

@@ -230,6 +230,10 @@ impl BlobTree {
 }
 
 impl AbstractTree for BlobTree {
+    fn l0_run_count(&self) -> usize {
+        self.index.l0_run_count()
+    }
+
     fn blob_file_count(&self) -> usize {
         self.blobs.segment_count()
     }
@@ -256,10 +260,6 @@ impl AbstractTree for BlobTree {
         self.index.sealed_memtable_count()
     }
 
-    fn is_first_level_disjoint(&self) -> bool {
-        self.index.is_first_level_disjoint()
-    }
-
     #[doc(hidden)]
     fn verify(&self) -> crate::Result<usize> {
         let index_tree_sum = self.index.verify()?;

src/blob_tree/value.rs

@@ -4,7 +4,7 @@
 
 use crate::coding::{Decode, DecodeError, Encode, EncodeError};
 use byteorder::{ReadBytesExt, WriteBytesExt};
-use std::io::{Read, Write};
+use std::io::{Cursor, Read, Write};
 use value_log::{Slice, UserValue, ValueHandle};
 use varint_rs::{VarintReader, VarintWriter};
 
@@ -22,46 +22,25 @@ pub enum MaybeInlineValue {
     Indirect { vhandle: ValueHandle, size: u32 },
 }
 
-impl Encode for ValueHandle {
-    fn encode_into<W: Write>(&self, writer: &mut W) -> Result<(), EncodeError> {
-        writer.write_u64_varint(self.offset)?;
-        writer.write_u64_varint(self.segment_id)?;
-        Ok(())
-    }
-}
-
-impl Decode for ValueHandle {
-    fn decode_from<R: Read>(reader: &mut R) -> Result<Self, DecodeError> {
-        let offset = reader.read_u64_varint()?;
-        let segment_id = reader.read_u64_varint()?;
-        Ok(Self { segment_id, offset })
-    }
-}
-
 const TAG_INLINE: u8 = 0;
 const TAG_INDIRECT: u8 = 1;
 
 impl MaybeInlineValue {
     pub fn from_slice(bytes: &Slice) -> Result<Self, DecodeError> {
-        let tag = *bytes.first().expect("vhandle bytes should not be empty");
+        let mut cursor = Cursor::new(&**bytes);
 
-        match tag {
+        match cursor.read_u8()? {
             TAG_INLINE => {
-                // TODO: 3.0.0 because the length field is varint encoded
-                // TODO: we need to get the amount of bytes the integer needs
-                // TODO: maybe not use varint encoding here... (breaking)
-                let len_size = {
-                    let mut reader = bytes.get(1..).expect("see above");
-                    let len = reader.read_u32_varint()?;
-                    match len {
-                        0..=0x7F => 1,                  // Fits in 7 bits
-                        0x80..=0x3FFF => 2,             // Fits in 14 bits
-                        0x4000..=0x001F_FFFF => 3,      // Fits in 21 bits
-                        0x0020_0000..=0x0FFF_FFFF => 4, // Fits in 28 bits
-                        _ => 5,                         // Fits in 35 bits
-                    }
+                // NOTE: Truncation is OK because we are only at the first couple
+                // of bytes of the slice
+                #[allow(clippy::cast_possible_truncation)]
+                let size_len = {
+                    let pos_before = cursor.position() as usize;
+                    let _ = cursor.read_u32_varint()?;
+                    let pos_after = cursor.position() as usize;
+                    pos_after - pos_before
                 };
-                let slice = bytes.slice((1 + len_size)..);
+                let slice = bytes.slice((1 + size_len)..);
                 Ok(Self::Inline(slice))
             }
             TAG_INDIRECT => {

src/block_cache.rs

@@ -3,8 +3,8 @@
 // (found in the LICENSE-* files in the repository)
 
 use crate::either::Either::{self, Left, Right};
+use crate::segment::block::offset::BlockOffset;
 use crate::segment::id::GlobalSegmentId;
-use crate::segment::value_block::BlockOffset;
 use crate::segment::{block_index::IndexBlock, value_block::ValueBlock};
 use quick_cache::Weighter;
 use quick_cache::{sync::Cache, Equivalent};

src/coding.rs

@@ -122,17 +122,16 @@ mod tests {
         config::Config,
         descriptor_table::FileDescriptorTable,
         file::LEVELS_MANIFEST_FILE,
-        key_range::KeyRange,
         level_manifest::LevelManifest,
         segment::{
+            block::offset::BlockOffset,
             block_index::{two_level_index::TwoLevelBlockIndex, BlockIndexImpl},
             file_offsets::FileOffsets,
             meta::{Metadata, SegmentId},
-            value_block::BlockOffset,
             Segment, SegmentInner,
         },
         time::unix_timestamp,
-        HashSet,
+        HashSet, KeyRange,
     };
     use std::sync::Arc;
     use test_log::test;

src/compaction/fifo.rs

@@ -5,11 +5,10 @@
 use super::{Choice, CompactionStrategy, Input as CompactionInput};
 use crate::{
     config::Config,
-    key_range::KeyRange,
     level_manifest::{hidden_set::HiddenSet, level::Level, LevelManifest},
     segment::Segment,
     windows::{GrowingWindowsExt, ShrinkingWindowsExt},
-    HashSet, SegmentId,
+    HashSet, KeyRange, SegmentId,
 };
 
 // TODO: for a disjoint set of segments, we could just take the first and last segment and use their first and last key respectively
@@ -387,17 +386,16 @@ mod tests {
         block_cache::BlockCache,
         compaction::{CompactionStrategy, Input as CompactionInput},
         descriptor_table::FileDescriptorTable,
-        key_range::KeyRange,
         level_manifest::LevelManifest,
         segment::{
+            block::offset::BlockOffset,
             block_index::{two_level_index::TwoLevelBlockIndex, BlockIndexImpl},
             file_offsets::FileOffsets,
             meta::{Metadata, SegmentId},
-            value_block::BlockOffset,
             Segment, SegmentInner,
         },
         time::unix_timestamp,
-        Config, HashSet,
+        Config, HashSet, KeyRange,
     };
     use std::{path::Path, sync::Arc};
     use test_log::test;

src/compaction/leveled.rs

@@ -87,15 +87,15 @@ mod tests {
         config::Config,
         descriptor_table::FileDescriptorTable,
         file::LEVELS_MANIFEST_FILE,
-        key_range::KeyRange,
         level_manifest::LevelManifest,
         segment::{
+            block::offset::BlockOffset,
             block_index::{two_level_index::TwoLevelBlockIndex, BlockIndexImpl},
             file_offsets::FileOffsets,
             meta::Metadata,
-            value_block::BlockOffset,
             Segment, SegmentInner,
         },
+        KeyRange,
     };
     use std::sync::Arc;
     use test_log::test;

src/compaction/maintenance.rs

@@ -136,16 +136,15 @@ mod tests {
         config::Config,
         descriptor_table::FileDescriptorTable,
         file::LEVELS_MANIFEST_FILE,
-        key_range::KeyRange,
         level_manifest::LevelManifest,
         segment::{
+            block::offset::BlockOffset,
             block_index::{two_level_index::TwoLevelBlockIndex, BlockIndexImpl},
             file_offsets::FileOffsets,
             meta::{Metadata, SegmentId},
-            value_block::BlockOffset,
             Segment, SegmentInner,
         },
-        HashSet, SeqNo,
+        HashSet, KeyRange, SeqNo,
     };
     use std::sync::Arc;
     use test_log::test;