diff --git a/turbopack/crates/turbo-persistence/README.md b/turbopack/crates/turbo-persistence/README.md index 52c0f31305cc..79514f3a2f4d 100644 --- a/turbopack/crates/turbo-persistence/README.md +++ b/turbopack/crates/turbo-persistence/README.md @@ -118,39 +118,40 @@ The hashes are sorted. - 1 byte block type (1: key block with hash, 2: key block without hash) - 3 bytes entry count -- foreach entry +- offset table: foreach entry + - 8 bytes key hash (block type 1 only) - 1 byte type - 3 bytes position in block after header - Max block size: 16 KB A Key block contains n keys, which specify n key value pairs. -The block type determines whether the key hash is stored per entry, and with it the order the -entries are stored in: +The block type determines whether the key hash is stored, and with it the order the entries are +stored in: -- Block type 1 (with hash): Full 8-byte hash stored per entry. Entries are sorted by - `(key hash, key)`. +- Block type 1 (with hash): Full 8-byte hash per entry, stored in the offset table. Entries are + sorted by `(key hash, key)`. - Block type 2 (no hash): No hash stored (for keys ≤ 32 bytes). Entries are sorted by **key**. See [Entry ordering](#entry-ordering) for why the two differ. +The hash lives in the offset table rather than beside its key so that a lookup's binary search reads +only that dense array. Fixed-size key blocks apply the same idea; see +[Two regions, not interleaved](#two-regions-not-interleaved). + Depending on the `type` field entry has a different format: - 0: normal key (small value) - - 8 bytes key hash (if block type 1) - key data - 2 byte block index - 2 bytes size - 4 bytes position in block - 1: blob reference - - 8 bytes key hash (if block type 1) - key data - 4 bytes sequence number - 2: deleted key / key tombstone (no data) - - 8 bytes key hash (if block type 1) - key data - 3: normal key (medium sized value) - - 8 bytes key hash (if block type 1) - key data - 2 byte block index - 7: merge key (future) @@ -160,12 +161,10 @@ Depending on the `type` field entry has a different format: - 4 bytes position in block - 8..=16: inlined value, size = type - 8 (the format supports up to 247, but `MAX_INLINE_VALUE_SIZE` currently caps it at 8) - - 8 bytes key hash (if block type 1) - key data - (type - 8) bytes value data (inline, no separate value block) - 17..=25: key-value tombstone, deleted value size = type - 17 (mirrors the inline range and shifts with `MAX_INLINE_VALUE_SIZE`) - - 8 bytes key hash (if block type 1) - key data - (type - 17) bytes of the deleted value, stored inline @@ -174,7 +173,7 @@ the inline range. ##### Entry ordering -Logically keys are ordered by hash (this is how we chose file and block assignments). However, within a single key block, however, the order is chosen per block type: +Logically keys are ordered by hash (this is how we chose file and block assignments). However, within a key block, the order may be different - **With hash (types 1 and 3):** sorted by `(key hash, key)`. - **No hash (types 2 and 4):** sorted by **key** alone. @@ -211,9 +210,10 @@ during binary search. - 1 byte value type (shared by all entries, same encoding as variable-size type field), or `FIXED_KEY_BLOCK_MIXED_VALUE_TYPE` (4) when entries share a value size but not a value type - 1 byte value size — only present when the value type is `FIXED_KEY_BLOCK_MIXED_VALUE_TYPE` -- foreach entry (packed at stride = hash_len + key_size + val_size): - - 8 bytes key hash (if block type 3) - - key data (key_size bytes) +- search region, foreach entry at stride `search_stride`: + - 8 bytes key hash (block type 3), or key data (block type 4, `key_size` bytes) +- tail region, foreach entry at stride `tail_stride`: + - key data (block type 3 only, `key_size` bytes) - 1 byte value type — only present when the block is mixed-type - value data (size determined by the block's or the entry's value type) @@ -221,7 +221,21 @@ The mixed-type form exists so that same-sized inline values and key-value tombst fixed-size block: they have equal value sizes but different type bytes. Tag 4 is available as the mixed marker because it is not itself a valid entry type. -Entry position for index `i` is computed as `header_size + i * stride` with no indirection. The writer automatically selects fixed-size format when all entries in a block qualify; otherwise falls back to the variable-size format above. +##### Two regions, not interleaved + +Rather than one interleaved record per entry, entries are split into a **search region** and a +**tail region**, indexed by the same entry number. The search region holds only the bytes binary +search compares first — the hash for block type `3`, the key for block type `4` (see +[Entry ordering](#entry-ordering)) — so a probe searches the dense prefix region. The _values_ are then found by index in the **tail** +after the search succeeds. + +Entry positions for index `i` are `header_size + i * search_stride` and +`header_size + entry_count * search_stride + i * tail_stride`, both with no indirection. + +The search region must be in ascending order for that binary search to be valid. This is inherited +from the `(key hash, key)` order the writer requires of its input, not established per block, so +reordering entries within a block is a format violation rather than a free choice — see +[Entry ordering](#entry-ordering). #### Value Block diff --git a/turbopack/crates/turbo-persistence/benches/mod.rs b/turbopack/crates/turbo-persistence/benches/mod.rs index 1f5e839e9bcc..5ec4a29069da 100644 --- a/turbopack/crates/turbo-persistence/benches/mod.rs +++ b/turbopack/crates/turbo-persistence/benches/mod.rs @@ -1201,8 +1201,8 @@ fn bench_static_sorted_file_lookup(c: &mut Criterion) { }) .collect(); - // Sort by hash (required by write_static_stored_file) - entries.sort_by_key(|e| e.hash); + // Sort by (hash, key) order, as required by write_static_stored_file + entries.sort_by_key(|e| (e.hash, e.key)); // Create temp directory and write SST file let tempdir = tempfile::tempdir().unwrap(); diff --git a/turbopack/crates/turbo-persistence/src/arc_bytes.rs b/turbopack/crates/turbo-persistence/src/arc_bytes.rs index 09e189e9a307..8b32bf939366 100644 --- a/turbopack/crates/turbo-persistence/src/arc_bytes.rs +++ b/turbopack/crates/turbo-persistence/src/arc_bytes.rs @@ -11,26 +11,44 @@ use memmap2::Mmap; use crate::{ Compression, compression::decompress_into_arc, - shared_bytes::{SharedBytes, is_subslice_of}, + shared_bytes::{INLINE_CAPACITY, SharedBytes, is_subslice_of}, }; -/// The backing storage for an `ArcBytes`. +/// The representation of an `ArcBytes`. /// -/// The inner values are never read directly — they exist solely to keep the -/// backing memory alive while the raw `data` pointer in `ArcBytes` references it. +/// For the ref-counted variants the handle is never read directly — it exists solely to keep the +/// backing memory alive while `data` points into it. `Inline` instead owns its bytes, so it has no +/// `data` pointer: one would dangle as soon as the value moved. #[derive(Clone)] -enum Backing { - Arc { _backing: Arc<[u8]> }, - Mmap { _backing: Arc }, +enum Repr { + Arc { + data: *const [u8], + _backing: Arc<[u8]>, + }, + Mmap { + data: *const [u8], + _backing: Arc, + }, + /// Bytes stored in place, for slices up to [`INLINE_CAPACITY`]. + Inline { buf: [u8; INLINE_CAPACITY], len: u8 }, } -/// An owned byte slice backed by either an `Arc<[u8]>` or a memory-mapped file. +/// An owned byte slice backed by an `Arc<[u8]>`, a memory-mapped file, or — for short slices — an +/// inline buffer that avoids touching a refcount at all. #[derive(Clone)] pub struct ArcBytes { - data: *const [u8], - // Safety: Backing should come last so that it is dropped after the data pointer so we don't - // create a dangling pointer. This isn't really a problem since it is technically ok to have - // dangling _pointers_. - backing: Backing, + repr: Repr, +} + +impl ArcBytes { + /// The ref-counted bytes this slice points into, or `None` when stored inline. + #[inline] + fn backing_bytes(&self) -> Option<&[u8]> { + match &self.repr { + Repr::Arc { _backing, .. } => Some(_backing), + Repr::Mmap { _backing, .. } => Some(_backing), + Repr::Inline { .. } => None, + } + } } unsafe impl Send for ArcBytes {} @@ -39,8 +57,10 @@ unsafe impl Sync for ArcBytes {} impl From> for ArcBytes { fn from(arc: Arc<[u8]>) -> Self { Self { - data: &*arc as *const [u8], - backing: Backing::Arc { _backing: arc }, + repr: Repr::Arc { + data: &*arc as *const [u8], + _backing: arc, + }, } } } @@ -55,7 +75,13 @@ impl Deref for ArcBytes { type Target = [u8]; fn deref(&self) -> &Self::Target { - unsafe { &*self.data } + match &self.repr { + // SAFETY: `data` points into the backing held by the same variant, which keeps it + // alive for as long as `self`. + Repr::Arc { data, .. } | Repr::Mmap { data, .. } => unsafe { &**data }, + // Borrowed from `self`, so this is recomputed after a move rather than stored. + Repr::Inline { buf, len } => &buf[..*len as usize], + } } } @@ -88,7 +114,7 @@ impl Eq for ArcBytes {} impl ArcBytes { /// Returns `true` if this `ArcBytes` is backed by a memory-mapped file. pub fn is_mmap_backed(&self) -> bool { - matches!(self.backing, Backing::Mmap { .. }) + matches!(self.repr, Repr::Mmap { .. }) } /// Returns `true` if the backing `Arc` allocation is shared (i.e., there @@ -96,9 +122,9 @@ impl ArcBytes { /// Always returns `false` for mmap-backed bytes, since the mmap `Arc` is /// shared across all slices from the same file and is not a useful signal. pub fn is_shared_arc(&self) -> bool { - match &self.backing { - Backing::Arc { _backing } => Arc::strong_count(_backing) > 1, - Backing::Mmap { .. } => false, + match &self.repr { + Repr::Arc { _backing, .. } => Arc::strong_count(_backing) > 1, + Repr::Mmap { .. } | Repr::Inline { .. } => false, } } } @@ -107,28 +133,49 @@ impl SharedBytes for ArcBytes { type MmapHandle = Arc; fn slice(self, range: Range) -> Self { - let data = &*self; - let data = &data[range] as *const [u8]; + let sliced = &self[range]; + // Inline bytes have no backing to carry over, so re-inline the sub-range. + if let Repr::Inline { .. } = self.repr { + return Self::from_inline(sliced); + } + let data = sliced as *const [u8]; Self { - data, - backing: self.backing, + repr: match self.repr { + Repr::Arc { _backing, .. } => Repr::Arc { data, _backing }, + Repr::Mmap { _backing, .. } => Repr::Mmap { data, _backing }, + Repr::Inline { .. } => unreachable!("handled above"), + }, } } unsafe fn slice_from_subslice(&self, subslice: &[u8]) -> Self { + // Short slices are copied instead of pointed at, so the result owns its bytes and the + // caller's backing can be dropped. This is the common case on the lookup path: an inline + // value or a key-value tombstone payload, both of which live in a key block, so copying + // here is what lets a lookup avoid keeping that block alive. + if subslice.len() <= INLINE_CAPACITY { + return Self::from_inline(subslice); + } debug_assert!( - is_subslice_of( - subslice, - match &self.backing { - Backing::Arc { _backing } => _backing, - Backing::Mmap { _backing } => _backing, - } - ), + self.backing_bytes() + .is_some_and(|backing| is_subslice_of(subslice, backing)), "slice_from_subslice: subslice is not within the backing storage" ); + let data = subslice as *const [u8]; Self { - data: subslice as *const [u8], - backing: self.backing.clone(), + repr: match &self.repr { + Repr::Arc { _backing, .. } => Repr::Arc { + data, + _backing: _backing.clone(), + }, + Repr::Mmap { _backing, .. } => Repr::Mmap { + data, + _backing: _backing.clone(), + }, + // Unreachable for a well-formed caller: an inline slice is at most + // INLINE_CAPACITY, so it took the branch above. + Repr::Inline { .. } => return Self::from_inline(subslice), + }, } } @@ -138,8 +185,8 @@ impl SharedBytes for ArcBytes { "from_mmap: subslice is not within the mmap" ); ArcBytes { - data: subslice as *const [u8], - backing: Backing::Mmap { + repr: Repr::Mmap { + data: subslice as *const [u8], _backing: mmap.clone(), }, } @@ -156,4 +203,21 @@ impl SharedBytes for ArcBytes { block, )?)) } + + #[inline] + fn from_inline(bytes: &[u8]) -> Self { + assert!( + bytes.len() <= INLINE_CAPACITY, + "{} bytes exceeds the {INLINE_CAPACITY} byte inline capacity", + bytes.len() + ); + let mut buf = [0u8; INLINE_CAPACITY]; + buf[..bytes.len()].copy_from_slice(bytes); + ArcBytes { + repr: Repr::Inline { + buf, + len: bytes.len() as u8, + }, + } + } } diff --git a/turbopack/crates/turbo-persistence/src/bin/sst_inspect.rs b/turbopack/crates/turbo-persistence/src/bin/sst_inspect.rs index 740f6b4757bc..72e760fc40a9 100644 --- a/turbopack/crates/turbo-persistence/src/bin/sst_inspect.rs +++ b/turbopack/crates/turbo-persistence/src/bin/sst_inspect.rs @@ -26,11 +26,11 @@ use turbo_persistence::{ read_current_version, sst_filter::SstFilter, static_sorted_file::{ - BLOCK_TYPE_FIXED_KEY_NO_HASH, BLOCK_TYPE_FIXED_KEY_WITH_HASH, BLOCK_TYPE_KEY_NO_HASH, - BLOCK_TYPE_KEY_WITH_HASH, FIXED_KEY_BLOCK_MIXED_VALUE_TYPE, KEY_BLOCK_ENTRY_TYPE_BLOB, + FIXED_KEY_BLOCK_MIXED_VALUE_TYPE, FixedRegions, KEY_BLOCK_ENTRY_TYPE_BLOB, KEY_BLOCK_ENTRY_TYPE_INLINE_MIN, KEY_BLOCK_ENTRY_TYPE_KEY_DELETED, KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN, KEY_BLOCK_ENTRY_TYPE_MEDIUM, - KEY_BLOCK_ENTRY_TYPE_SMALL, + KEY_BLOCK_ENTRY_TYPE_SMALL, KEY_BLOCK_TABLE_ENTRY_SIZE_NO_HASH, KeyBlockLayout, + key_block_table_stride, }, }; @@ -394,18 +394,19 @@ fn parse_key_block_indices(index_block: &[u8]) -> HashSet { enum KeyBlockHeader { Variable { entry_count: u32, + /// Bytes per offset table entry, wider when the block hoists hashes into the table. + table_stride: usize, }, Fixed { entry_count: u32, value_type: u8, }, /// Fixed-size layout whose entries share a value size but not a value type, so each carries - /// its own type byte between its key and its value. + /// its own type byte ahead of its value in the block's tail region. FixedMixedType { entry_count: u32, - hash_len: usize, - key_size: usize, - stride: usize, + /// Where the block's search and tail regions sit, derived by the shared reader helper. + regions: FixedRegions, }, } @@ -414,36 +415,34 @@ fn parse_key_block_header(block: &[u8]) -> Result { assert!(block.len() >= 4, "Key block too small"); let block_type = block[0]; let entry_count = ((block[1] as u32) << 16) | ((block[2] as u32) << 8) | (block[3] as u32); - match block_type { - BLOCK_TYPE_KEY_WITH_HASH | BLOCK_TYPE_KEY_NO_HASH => { - Ok(KeyBlockHeader::Variable { entry_count }) - } - BLOCK_TYPE_FIXED_KEY_WITH_HASH | BLOCK_TYPE_FIXED_KEY_NO_HASH => { - assert!(block.len() >= 6, "Fixed key block header too small"); - if block[5] == FIXED_KEY_BLOCK_MIXED_VALUE_TYPE { - assert!(block.len() >= 7, "Mixed-type key block header too small"); - let hash_len = if block_type == BLOCK_TYPE_FIXED_KEY_WITH_HASH { - 8 - } else { - 0 - }; - let key_size = block[4] as usize; - let val_size = block[6] as usize; - Ok(KeyBlockHeader::FixedMixedType { - entry_count, - hash_len, - key_size, - // +1 for the per-entry type byte. - stride: hash_len + key_size + val_size + 1, - }) - } else { - Ok(KeyBlockHeader::Fixed { - entry_count, - value_type: block[5], - }) - } - } - _ => bail!("Invalid key block type: {block_type}"), + let Some((layout, fixed)) = KeyBlockLayout::from_block_type(block_type) else { + bail!("Invalid key block type: {block_type}"); + }; + if !fixed { + return Ok(KeyBlockHeader::Variable { + entry_count, + table_stride: key_block_table_stride(layout.hash_len()), + }); + } + assert!(block.len() >= 6, "Fixed key block header too small"); + if block[5] == FIXED_KEY_BLOCK_MIXED_VALUE_TYPE { + assert!(block.len() >= 7, "Mixed-type key block header too small"); + Ok(KeyBlockHeader::FixedMixedType { + entry_count, + // `FixedRegions` owns the search/tail split; `val_size` includes the per-entry type + // byte, which the header stores separately from the value size. + regions: FixedRegions::new( + entry_count as usize, + layout, + block[4] as usize, + block[6] as usize + 1, + ), + }) + } else { + Ok(KeyBlockHeader::Fixed { + entry_count, + value_type: block[5], + }) } } @@ -457,24 +456,23 @@ fn iter_key_block_entry_types( block: &[u8], ) -> impl Iterator + '_ { let entry_count = match header { - KeyBlockHeader::Variable { entry_count } + KeyBlockHeader::Variable { entry_count, .. } | KeyBlockHeader::Fixed { entry_count, .. } | KeyBlockHeader::FixedMixedType { entry_count, .. } => entry_count, }; (0..entry_count).map(move |i| match header { - // Variable block: offset table starts at byte 4 (after 1B type + 3B count), - // each entry is 4 bytes, first byte is the entry type. - KeyBlockHeader::Variable { .. } => block[KEY_BLOCK_HEADER_SIZE + i as usize * 4], + // Variable block: offset table starts at byte 4 (after 1B type + 3B count). The type byte + // leads the trailing type/position word, which follows any hoisted hash. + KeyBlockHeader::Variable { table_stride, .. } => { + block[KEY_BLOCK_HEADER_SIZE + + i as usize * table_stride + + (table_stride - KEY_BLOCK_TABLE_ENTRY_SIZE_NO_HASH)] + } KeyBlockHeader::Fixed { value_type, .. } => value_type, - KeyBlockHeader::FixedMixedType { - hash_len, - key_size, - stride, - .. - } => { - // Entry data starts after the 7-byte mixed-type header; the type byte sits between - // the entry's key and its value. - block[7 + i as usize * stride + hash_len + key_size] + KeyBlockHeader::FixedMixedType { regions, .. } => { + // Entry data starts after the 7-byte mixed-type header; within the tail region the + // type byte precedes the value, after the key for `HashThenKey` blocks. + block[7 + regions.total_len(i as usize) + regions.tail_key_size()] } }) } diff --git a/turbopack/crates/turbo-persistence/src/db.rs b/turbopack/crates/turbo-persistence/src/db.rs index 72d5a3a3acf3..4240feae144f 100644 --- a/turbopack/crates/turbo-persistence/src/db.rs +++ b/turbopack/crates/turbo-persistence/src/db.rs @@ -986,9 +986,9 @@ impl TurboPersistence let ssts = meta .entries() .iter() - .map(|entry| { + .zip(meta.hash_ranges()) + .map(|(entry, range)| { let seq = entry.sequence_number(); - let range = entry.range(); let size = entry.size(); let flags = entry.flags(); (seq, range.min_hash, range.max_hash, size, flags) @@ -1273,9 +1273,8 @@ impl TurboPersistence continue; } let meta_seq = meta.sequence_number(); - for entry in meta.entries().iter() { + for (entry, range) in meta.entries().iter().zip(meta.hash_ranges()) { let seq = entry.sequence_number(); - let range = entry.range(); writeln!( log, "{family:3} | {meta_seq:08} | {seq:08} {:>6} | {}", @@ -1386,6 +1385,7 @@ impl TurboPersistence struct SstWithRange { meta_index: usize, + family: u32, index_in_meta: u32, seq: u32, range: StaticSortedFileRange, @@ -1418,9 +1418,10 @@ impl TurboPersistence .enumerate() .map(move |(index_in_meta, entry)| SstWithRange { meta_index, + family: meta.family(), index_in_meta: index_in_meta as u32, seq: entry.sequence_number(), - range: entry.range(), + range: meta.range(index_in_meta as u32), size: entry.size(), flags: entry.flags(), }) @@ -1430,7 +1431,7 @@ impl TurboPersistence let mut sst_by_family = [(); FAMILIES].map(|_| Vec::new()); for sst in ssts_with_ranges { - sst_by_family[sst.range.family as usize].push(sst); + sst_by_family[sst.family as usize].push(sst); } let path = &self.path; @@ -1556,9 +1557,10 @@ impl TurboPersistence let meta_file = &meta_files[meta_index]; let entry = meta_file.entry(index_in_meta); let amqf = Cow::Borrowed(entry.raw_amqf(meta_file.amqf_data())); + let hash_range = meta_file.hash_range(index_in_meta); let meta = StaticSortedFileBuilderMeta { - min_hash: entry.min_hash(), - max_hash: entry.max_hash(), + min_hash: hash_range.min_hash, + max_hash: hash_range.max_hash, amqf, block_count: entry.block_count(), size: entry.size(), @@ -1591,9 +1593,10 @@ impl TurboPersistence let older_filters = ssts_with_ranges[..oldest_index_in_job] .iter() .map(|sst| { - let entry = - meta_files[sst.meta_index].entry(sst.index_in_meta); - (entry.min_hash(), entry.max_hash(), entry.amqf()) + let meta_file = &meta_files[sst.meta_index]; + let entry = meta_file.entry(sst.index_in_meta); + let range = meta_file.hash_range(sst.index_in_meta); + (range.min_hash, range.max_hash, entry.amqf()) }) .collect::>(); move |hash: u64| { @@ -2058,13 +2061,15 @@ impl TurboPersistence let mut size = 0; + let key_block_cache = self.key_block_cache(); + let value_block_cache = self.value_block_cache(); for meta in inner.meta_files.iter().rev() { match meta.lookup::( family as u32, hash, key, - self.key_block_cache(), - self.value_block_cache(), + key_block_cache, + value_block_cache, )? { MetaLookupResult::FamilyMiss => { #[cfg(feature = "stats")] @@ -2195,14 +2200,16 @@ impl TurboPersistence } cells.sort_by_key(|(hash, _, _)| *hash); let inner = self.inner.read(); + let key_block_cache = self.key_block_cache(); + let value_block_cache = self.value_block_cache(); for meta in inner.meta_files.iter().rev() { let _result = meta.batch_lookup( family as u32, keys, &mut cells, &mut empty_cells, - self.key_block_cache(), - self.value_block_cache(), + key_block_cache, + value_block_cache, )?; #[cfg(feature = "stats")] @@ -2321,12 +2328,13 @@ impl TurboPersistence let entries = meta_file .entries() .iter() - .map(|entry| { + .zip(meta_file.hash_ranges()) + .map(|(entry, range)| { let amqf = entry.raw_amqf(meta_file.amqf_data()); MetaFileEntryInfo { sequence_number: entry.sequence_number(), - min_hash: entry.min_hash(), - max_hash: entry.max_hash(), + min_hash: range.min_hash, + max_hash: range.max_hash, sst_size: entry.size(), flags: entry.flags(), amqf_size: entry.amqf_size(), diff --git a/turbopack/crates/turbo-persistence/src/meta_file.rs b/turbopack/crates/turbo-persistence/src/meta_file.rs index 0a288ceb284f..3ed7376530df 100644 --- a/turbopack/crates/turbo-persistence/src/meta_file.rs +++ b/turbopack/crates/turbo-persistence/src/meta_file.rs @@ -1,6 +1,7 @@ use std::{ cmp::Ordering, fmt::Display, + mem::take, ops::Deref, path::{Path, PathBuf}, sync::OnceLock, @@ -93,19 +94,15 @@ impl EntryHeader { /// # Safety /// /// `MetaEntry` stores a `FilterRef<'static>` with a transmuted lifetime that actually borrows -/// from the parent [`MetaFile`]'s stable backing bytes. This is safe because entries are only -/// accessed by reference through `MetaFile` and are never moved out. +/// from the parent [`MetaFile`]'s stable backing bytes. This is safe as long as an entry never +/// outlives that backing: entries are only handed out by reference, and the one place that moves +/// them ([`MetaFile::retain_entries`]) keeps them inside the same `MetaFile`. /// -/// For this reason this type should not implement Clone or Copy. +/// For this reason this type should not implement Clone or Copy — a copy could outlive the +/// `MetaFile` that owns the backing it points into. pub struct MetaEntry { /// The metadata for the static sorted file. sst_data: StaticSortedFileMetaData, - /// The key family of the SST file. - family: u32, - /// The minimum hash value of the keys in the SST file. - min_hash: u64, - /// The maximum hash value of the keys in the SST file. - max_hash: u64, /// The size of the SST file in bytes. size: u64, /// The status flags for this entry. @@ -171,23 +168,6 @@ impl MetaEntry { }) } - /// Returns the key family and hash range of this file. - pub fn range(&self) -> StaticSortedFileRange { - StaticSortedFileRange { - family: self.family, - min_hash: self.min_hash, - max_hash: self.max_hash, - } - } - - pub fn min_hash(&self) -> u64 { - self.min_hash - } - - pub fn max_hash(&self) -> u64 { - self.max_hash - } - pub fn block_count(&self) -> u16 { self.sst_data.block_count } @@ -236,11 +216,18 @@ pub struct MetaBatchLookupResult { /// The key family and hash range of an SST file. #[derive(Clone, Copy)] pub struct StaticSortedFileRange { - pub family: u32, pub min_hash: u64, pub max_hash: u64, } +impl StaticSortedFileRange { + /// Whether `hash` falls within this file's span. A lookup can skip the file entirely if not. + #[inline(always)] + pub fn contains(&self, hash: u64) -> bool { + hash >= self.min_hash && hash <= self.max_hash + } +} + enum MetaFileBacking { Mmap(Mmap), Bytes(Box<[u8]>), @@ -270,8 +257,11 @@ pub struct MetaFile { family: u32, /// Compression recorded for this family. compression: Compression, + /// Stored separately from [`MetaEntry`] so that lookups can operate over a denser data + /// structure that's hotter in cache. + hash_ranges: Box<[StaticSortedFileRange]>, /// The entries of the file. Dropped before `backing` (field declaration order). - entries: Vec, + entries: Box<[MetaEntry]>, /// The entries that have been marked as obsolete. obsolete_entries: Vec, /// The obsolete SST files. @@ -369,6 +359,7 @@ impl MetaFile { // Parse entries and eagerly deserialize AMQF filters as zero-copy FilterRefs. let mut entries = Vec::with_capacity(count as usize); + let mut hash_ranges = Vec::with_capacity(count as usize); let mut start_of_amqf_data_offset: u32 = 0; for _ in 0..count { let (header, rest): (Ref<&[u8], EntryHeader>, _) = Ref::from_prefix(reader) @@ -400,11 +391,9 @@ impl MetaFile { // declaration order), so the borrow remains valid for the lifetime of the MetaEntry. let amqf: qfilter::FilterRef<'static> = unsafe { std::mem::transmute(amqf) }; + hash_ranges.push(StaticSortedFileRange { min_hash, max_hash }); entries.push(MetaEntry { sst_data, - family, - min_hash, - max_hash, size, flags, amqf_data_offset: start_of_amqf_data_offset..end_of_amqf_data_offset, @@ -423,7 +412,8 @@ impl MetaFile { sequence_number, family, compression, - entries, + hash_ranges: hash_ranges.into_boxed_slice(), + entries: entries.into_boxed_slice(), obsolete_entries: Vec::new(), obsolete_sst_files, amqf_data_start, @@ -467,6 +457,21 @@ impl MetaFile { &self.entries } + /// The hash ranges of this file's entries, in the same order as [`Self::entries`]. + pub fn hash_ranges(&self) -> &[StaticSortedFileRange] { + &self.hash_ranges + } + + /// The hash range of the entry at `index`. + pub fn hash_range(&self, index: u32) -> StaticSortedFileRange { + self.hash_ranges[index as usize] + } + + /// The key family and hash range of the entry at `index`. + pub fn range(&self, index: u32) -> StaticSortedFileRange { + self.hash_range(index) + } + pub fn entry(&self, index: u32) -> &MetaEntry { let index = index as usize; &self.entries[index] @@ -491,15 +496,34 @@ impl MetaFile { } pub fn retain_entries(&mut self, mut predicate: impl FnMut(u32) -> bool) -> bool { + debug_assert_eq!( + self.entries.len(), + self.hash_ranges.len(), + "hash_ranges must stay parallel to entries" + ); let old_len = self.entries.len(); - self.entries.retain(|entry| { - if predicate(entry.sst_data.sequence_number) { - true - } else { - self.obsolete_entries.push(entry.sst_data.sequence_number); - false - } - }); + // Filter the two vectors as pairs so they cannot drift apart. Retaining them separately + // would leave a lookup indexing one by a position that means something else in the other. + // + // This rebuilds both vectors rather than compacting in place, which is the more expensive + // shape but a fine trade here: the callers are commit and compaction, never a lookup. + // + // Entries move between slots but never leave this `MetaFile`, so the `FilterRef`s they + // hold keep borrowing a mmap that is neither touched nor dropped. + let obsolete = &mut self.obsolete_entries; + let (entries, hash_ranges): (Vec<_>, Vec<_>) = take(&mut self.entries) + .into_iter() + .zip(take(&mut self.hash_ranges)) + .filter(|(entry, _)| { + let retain = predicate(entry.sst_data.sequence_number); + if !retain { + obsolete.push(entry.sst_data.sequence_number); + } + retain + }) + .unzip(); + self.entries = entries.into_boxed_slice(); + self.hash_ranges = hash_ranges.into_boxed_slice(); old_len != self.entries.len() } @@ -534,10 +558,11 @@ impl MetaFile { let mut miss_result = MetaLookupResult::RangeMiss; let mut all_results: SmallVec<[LookupValue; 1]> = SmallVec::new(); - for entry in self.entries.iter().rev() { - if key_hash < entry.min_hash || key_hash > entry.max_hash { + for (index, range) in self.hash_ranges.iter().enumerate().rev() { + if !range.contains(key_hash) { continue; } + let entry = &self.entries[index]; if !entry.amqf.contains_fingerprint(key_hash) { miss_result = MetaLookupResult::QuickFilterMiss; continue; @@ -608,9 +633,9 @@ impl MetaFile { ); #[allow(unused_mut, reason = "It's used when stats are enabled")] let mut lookup_result = MetaBatchLookupResult::default(); - for entry in self.entries.iter().rev() { + for (entry_index, range) in self.hash_ranges.iter().enumerate().rev() { let start_index = cells - .binary_search_by(|(hash, _, _)| hash.cmp(&entry.min_hash).then(Ordering::Greater)) + .binary_search_by(|(hash, _, _)| hash.cmp(&range.min_hash).then(Ordering::Greater)) .err() .unwrap(); if start_index >= cells.len() { @@ -621,7 +646,7 @@ impl MetaFile { continue; } let end_index = cells - .binary_search_by(|(hash, _, _)| hash.cmp(&entry.max_hash).then(Ordering::Less)) + .binary_search_by(|(hash, _, _)| hash.cmp(&range.max_hash).then(Ordering::Less)) .err() .unwrap() .checked_sub(1); @@ -639,11 +664,9 @@ impl MetaFile { } continue; } + let entry = &self.entries[entry_index]; for (hash, index, result) in &mut cells[start_index..=end_index] { - debug_assert!( - *hash >= entry.min_hash && *hash <= entry.max_hash, - "Key hash out of range" - ); + debug_assert!(range.contains(*hash), "Key hash out of range"); if result.is_some() { continue; } diff --git a/turbopack/crates/turbo-persistence/src/rc_bytes.rs b/turbopack/crates/turbo-persistence/src/rc_bytes.rs index c4c1007cab32..be380887b112 100644 --- a/turbopack/crates/turbo-persistence/src/rc_bytes.rs +++ b/turbopack/crates/turbo-persistence/src/rc_bytes.rs @@ -11,34 +11,56 @@ use memmap2::Mmap; use crate::{ Compression, compression::decompress_into_rc, - shared_bytes::{SharedBytes, is_subslice_of}, + shared_bytes::{INLINE_CAPACITY, SharedBytes, is_subslice_of}, }; -/// The backing storage for an `RcBytes`. +/// The representation of an `RcBytes`. /// -/// Uses `Rc` for all refcounting, eliminating atomic operations. +/// Mirrors [`ArcBytes`][crate::ArcBytes]: the ref-counted variants keep their backing alive while +/// `data` points into it, and `Inline` owns its bytes so it carries no pointer to dangle on a move. #[derive(Clone)] -enum Backing { - Rc { _backing: Rc<[u8]> }, - Mmap { _backing: Rc }, +enum Repr { + Rc { + data: *const [u8], + _backing: Rc<[u8]>, + }, + Mmap { + data: *const [u8], + _backing: Rc, + }, + /// Bytes stored in place, for slices up to [`INLINE_CAPACITY`]. + Inline { buf: [u8; INLINE_CAPACITY], len: u8 }, } -/// An owned byte slice backed by either an `Rc<[u8]>` or a memory-mapped file. +/// An owned byte slice backed by an `Rc<[u8]>`, a memory-mapped file, or an inline buffer. /// /// Identical to `ArcBytes` but uses `Rc` instead of `Arc`, eliminating atomic /// refcount overhead. Use this in single-threaded contexts like SST iteration /// during compaction. #[derive(Clone)] pub struct RcBytes { - data: *const [u8], - backing: Backing, + repr: Repr, +} + +impl RcBytes { + /// The ref-counted bytes this slice points into, or `None` when stored inline. + #[inline] + fn backing_bytes(&self) -> Option<&[u8]> { + match &self.repr { + Repr::Rc { _backing, .. } => Some(_backing), + Repr::Mmap { _backing, .. } => Some(_backing), + Repr::Inline { .. } => None, + } + } } impl From> for RcBytes { fn from(rc: Rc<[u8]>) -> Self { Self { - data: &*rc as *const [u8], - backing: Backing::Rc { _backing: rc }, + repr: Repr::Rc { + data: &*rc as *const [u8], + _backing: rc, + }, } } } @@ -53,7 +75,13 @@ impl Deref for RcBytes { type Target = [u8]; fn deref(&self) -> &Self::Target { - unsafe { &*self.data } + match &self.repr { + // SAFETY: `data` points into the backing held by the same variant, which keeps it + // alive for as long as `self`. + Repr::Rc { data, .. } | Repr::Mmap { data, .. } => unsafe { &**data }, + // Borrowed from `self`, so this is recomputed after a move rather than stored. + Repr::Inline { buf, len } => &buf[..*len as usize], + } } } @@ -87,28 +115,48 @@ impl SharedBytes for RcBytes { type MmapHandle = Rc; fn slice(self, range: Range) -> Self { - let data = &*self; - let data = &data[range] as *const [u8]; + let sliced = &self[range]; + // Inline bytes have no backing to carry over, so re-inline the sub-range. + if let Repr::Inline { .. } = self.repr { + return Self::from_inline(sliced); + } + let data = sliced as *const [u8]; Self { - data, - backing: self.backing, + repr: match self.repr { + Repr::Rc { _backing, .. } => Repr::Rc { data, _backing }, + Repr::Mmap { _backing, .. } => Repr::Mmap { data, _backing }, + Repr::Inline { .. } => unreachable!("handled above"), + }, } } unsafe fn slice_from_subslice(&self, subslice: &[u8]) -> Self { + // Mirrors `ArcBytes`: short slices are copied so the result owns its bytes. The refcount + // saved here is non-atomic and therefore cheap, but keeping the two types identical means + // the lookup and iteration paths cannot disagree about what a returned value borrows. + if subslice.len() <= INLINE_CAPACITY { + return Self::from_inline(subslice); + } debug_assert!( - is_subslice_of( - subslice, - match &self.backing { - Backing::Rc { _backing } => _backing, - Backing::Mmap { _backing } => _backing, - } - ), + self.backing_bytes() + .is_some_and(|backing| is_subslice_of(subslice, backing)), "slice_from_subslice: subslice is not within the backing storage" ); + let data = subslice as *const [u8]; Self { - data: subslice as *const [u8], - backing: self.backing.clone(), + repr: match &self.repr { + Repr::Rc { _backing, .. } => Repr::Rc { + data, + _backing: _backing.clone(), + }, + Repr::Mmap { _backing, .. } => Repr::Mmap { + data, + _backing: _backing.clone(), + }, + // Unreachable for a well-formed caller: an inline slice is at most + // INLINE_CAPACITY, so it took the branch above. + Repr::Inline { .. } => return Self::from_inline(subslice), + }, } } @@ -118,8 +166,8 @@ impl SharedBytes for RcBytes { "from_mmap: subslice is not within the mmap" ); RcBytes { - data: subslice as *const [u8], - backing: Backing::Mmap { + repr: Repr::Mmap { + data: subslice as *const [u8], _backing: mmap.clone(), }, } @@ -136,4 +184,21 @@ impl SharedBytes for RcBytes { block, )?)) } + + #[inline] + fn from_inline(bytes: &[u8]) -> Self { + assert!( + bytes.len() <= INLINE_CAPACITY, + "{} bytes exceeds the {INLINE_CAPACITY} byte inline capacity", + bytes.len() + ); + let mut buf = [0u8; INLINE_CAPACITY]; + buf[..bytes.len()].copy_from_slice(bytes); + RcBytes { + repr: Repr::Inline { + buf, + len: bytes.len() as u8, + }, + } + } } diff --git a/turbopack/crates/turbo-persistence/src/shared_bytes.rs b/turbopack/crates/turbo-persistence/src/shared_bytes.rs index 6dc0f8407d8f..92e887ff16cc 100644 --- a/turbopack/crates/turbo-persistence/src/shared_bytes.rs +++ b/turbopack/crates/turbo-persistence/src/shared_bytes.rs @@ -3,6 +3,21 @@ use std::ops::{Deref, Range}; use memmap2::Mmap; use crate::Compression; +/// Bytes that fit in this many bytes are stored directly inside an `ArcBytes`/`RcBytes` rather +/// than as a pointer into ref-counted backing storage. +/// +/// Sized to hold any inline value or key-value tombstone payload, which the writer caps at +/// [`MAX_INLINE_VALUE_SIZE`][crate::constants::MAX_INLINE_VALUE_SIZE]. Those are the only values +/// that live in a key block, so covering them means a lookup that returns one does not have to +/// keep the whole block alive. +pub(crate) const INLINE_CAPACITY: usize = 8; + +// Anything the writer can place in a key block must fit, or the common case would silently fall +// back to the pointer path. +const _: () = assert!( + INLINE_CAPACITY >= crate::constants::MAX_INLINE_VALUE_SIZE, + "INLINE_CAPACITY must cover every inline value the writer can emit" +); /// Trait abstracting over `ArcBytes` and `RcBytes`. /// @@ -43,6 +58,14 @@ pub trait SharedBytes: Clone + Deref + Sized { uncompressed_length: u32, block: &[u8], ) -> anyhow::Result; + + /// Copies `bytes` into an inline buffer, so the result owns them and borrows nothing. + /// + /// # Panics + /// + /// If `bytes.len() > INLINE_CAPACITY`. Callers reading a length off disk must bound it first — + /// [`entry_val_size`][crate::static_sorted_file] does this for key block entries. + fn from_inline(block: &[u8]) -> Self; } /// Returns `true` if `subslice` lies entirely within `backing`. diff --git a/turbopack/crates/turbo-persistence/src/static_sorted_file.rs b/turbopack/crates/turbo-persistence/src/static_sorted_file.rs index 9438ea64abf2..5107ab11922c 100644 --- a/turbopack/crates/turbo-persistence/src/static_sorted_file.rs +++ b/turbopack/crates/turbo-persistence/src/static_sorted_file.rs @@ -3,6 +3,7 @@ use std::{ cmp::Ordering, hash::BuildHasherDefault, io, + ops::Range, path::Path, rc::Rc, sync::{ @@ -28,7 +29,9 @@ use crate::{ mmap_helper::advise_mmap_for_persistence, rc_bytes::RcBytes, shared_bytes::SharedBytes, - static_sorted_file_builder::{BLOCK_HEADER_SIZE, INDEX_BLOCK_ENTRY_SIZE}, + static_sorted_file_builder::{ + BLOCK_HEADER_SIZE, INDEX_BLOCK_ENTRY_SIZE, INDEX_BLOCK_HEADER_SIZE, + }, }; /// The block header for an index block. @@ -111,6 +114,25 @@ pub const KEY_BLOCK_ENTRY_TYPE_INLINE_MIN: u8 = 8; pub const KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN: u8 = KEY_BLOCK_ENTRY_TYPE_INLINE_MIN + MAX_INLINE_VALUE_SIZE as u8 + 1; +/// Size of one variable-size key block offset table entry when the block stores no hash: +/// 1 byte entry type packed into the top of a 3-byte in-block position. +pub const KEY_BLOCK_TABLE_ENTRY_SIZE_NO_HASH: usize = 4; +/// Size of one variable-size key block offset table entry when the block stores a hash: the key's +/// 8-byte hash followed by the type/position word. +/// +/// The hash lives in the table rather than beside the key so that a binary search reads only this +/// dense array — [`compare_hash_key`] compares the hash first and reaches for the key only when two +/// hashes are equal, so the payload is touched once on a match and never on a miss. Total bytes are +/// unchanged: the table grows by 8 per entry and the payload shrinks by the same. +pub const KEY_BLOCK_TABLE_ENTRY_SIZE_WITH_HASH: usize = + KEY_BLOCK_TABLE_ENTRY_SIZE_NO_HASH + size_of::(); + +/// Bytes per offset table entry for a variable-size key block with the given hash length. +#[inline(always)] +pub fn key_block_table_stride(hash_len: u8) -> usize { + KEY_BLOCK_TABLE_ENTRY_SIZE_NO_HASH + hash_len as usize +} + /// Encoded size of a small value reference: 2B block index + 2B size + 4B offset. pub(crate) const SMALL_VALUE_REF_SIZE: usize = 8; /// Encoded size of a medium value reference: 2B block index. @@ -220,14 +242,18 @@ impl ValueBlockCache for ArcBlockCacheReader<'_> { block_index: u16, compression: Compression, ) -> Result { - get_or_cache_block( + // A value block's bytes are returned to the caller of `get`, so this one must own its + // handle. For an uncompressed mmap block that is the mmap refcount; for a compressed or + // file-backed one the cache entry's. + Ok(get_or_read_block( self.backing, meta, block_index, self.cache, self.verified_blocks, compression, - ) + )? + .into_owned(self.backing)) } fn read_uncached( @@ -308,10 +334,109 @@ pub struct StaticSortedFile { /// suffices: racing first-time verifications are idempotent. verified_blocks: Box<[AtomicU64]>, compression: Compression, + /// The index block, parsed once at open time. + index: IndexBlock, +} + +/// The index block of an SST file, resolved and validated once when the file is opened. +/// +/// Every lookup binary searches this one block, so everything that does not depend on the queried +/// hash is done here instead of per lookup: locating the block, verifying its CRC, checking the +/// block type, reading the first-child index, and splitting the entry array off the header. What +/// remains in [`StaticSortedFile::lookup_index_block`] is the search itself. +struct IndexBlock { + /// The `(hash, block index)` entry array that follows the 3-byte header, guaranteed to be a + /// whole number of entries. + entries: IndexEntries, + /// Block index for hashes below the first entry's hash. + first_block: u16, +} + +/// Where an [`IndexBlock`]'s entry array lives. +enum IndexEntries { + /// A byte range within the file's mmap. + /// + /// A range rather than a slice or an [`ArcBytes`]: a slice would make [`StaticSortedFile`] + /// borrow from its own `backing` field, and an `ArcBytes` would bump and drop the `mmap` + /// refcount on every lookup. All readers of a file share that one counter, so the contention + /// scales with reader threads — measured ~3 ns single-threaded but ~70 ns at 8 threads. + Mmap(Range), + /// Read into memory at open time, for the non-mmap backing, which has nothing to borrow from. + Owned(Box<[u8]>), +} + +impl IndexBlock { + /// Locates, verifies and parses the index block, which is always the file's last block. + fn parse(backing: &StaticSortedFileBacking, meta: &StaticSortedFileMetaData) -> Result { + ensure!( + meta.block_count > 0, + "{:08}.sst has no blocks, so no index block", + meta.sequence_number + ); + let block_index = meta.block_count - 1; + let (uncompressed_length, checksum, block) = get_raw_block(backing, meta, block_index) + .with_context(|| { + format!( + "Failed to read index block {} from {:08}.sst", + block_index, meta.sequence_number + ) + })?; + ensure!( + uncompressed_length == 0, + "index block {} of {:08}.sst is compressed, but index blocks are always written \ + uncompressed", + block_index, + meta.sequence_number + ); + // Verified here rather than through `verified_blocks`: this is the one and only read of + // this block's bytes, so the bitmap would never save any work for it. + let data = &*block; + verify_checksum(meta, data, checksum, block_index)?; + + ensure!( + data.len() >= INDEX_BLOCK_HEADER_SIZE, + "index block {} of {:08}.sst is too short ({} bytes)", + block_index, + meta.sequence_number, + data.len() + ); + ensure!( + be::read_u8(data) == BLOCK_TYPE_INDEX, + "block {} of {:08}.sst is the last block but not an index block (type {})", + block_index, + meta.sequence_number, + be::read_u8(data) + ); + let first_block = be::read_u16(&data[1..]); + let entry_bytes = &data[INDEX_BLOCK_HEADER_SIZE..]; + ensure!( + entry_bytes.len().is_multiple_of(INDEX_BLOCK_ENTRY_SIZE), + "index block {} of {:08}.sst has {} trailing bytes past its last entry", + block_index, + meta.sequence_number, + entry_bytes.len() % INDEX_BLOCK_ENTRY_SIZE + ); + + let entries = match backing { + // Store a range, not the slice: `StaticSortedFile` owns the mmap these bytes live in. + StaticSortedFileBacking::Mmap(mmap) => { + let start = entry_bytes.as_ptr() as usize - mmap.as_ptr() as usize; + IndexEntries::Mmap(start..start + entry_bytes.len()) + } + StaticSortedFileBacking::File { .. } => IndexEntries::Owned(entry_bytes.into()), + }; + Ok(Self { + entries, + first_block, + }) + } } impl StaticSortedFile { /// Opens an SST file using the configured access mode. + /// + /// Only the index block is read here, and its CRC is verified. Key and value blocks stay + /// lazy, read on demand. pub fn open( db_path: &Path, meta: StaticSortedFileMetaData, @@ -363,14 +488,43 @@ impl StaticSortedFile { let verified_blocks = (0..bitmap_words) .map(|_| AtomicU64::new(0)) .collect::>(); + + let index = IndexBlock::parse(&backing, &meta)?; + Ok(Self { meta, backing, verified_blocks, compression, + index, }) } + /// The index block's entry array: `(8-byte hash, 2-byte block index)` pairs, sorted by hash. + #[inline] + fn index_entries(&self) -> &[[u8; INDEX_BLOCK_ENTRY_SIZE]] { + let bytes = match (&self.index.entries, &self.backing) { + (IndexEntries::Mmap(range), StaticSortedFileBacking::Mmap(mmap)) => { + &mmap[range.clone()] + } + (IndexEntries::Owned(bytes), _) => &bytes[..], + // `IndexBlock::parse` only produces `Mmap` entries for an mmap backing, and the + // backing never changes after open. + (IndexEntries::Mmap(_), StaticSortedFileBacking::File { .. }) => unreachable!( + "mmap-ranged index entries with a file backing in {:08}.sst", + self.meta.sequence_number + ), + }; + debug_assert!( + bytes.len().is_multiple_of(INDEX_BLOCK_ENTRY_SIZE), + "index entry range is not entry-aligned" + ); + // SAFETY: `IndexBlock::parse` rejected the file unless the entry region's length was a + // multiple of `INDEX_BLOCK_ENTRY_SIZE`, and `entries` is fixed at that point, so the + // checked variant's remainder is always empty here. + unsafe { bytes.as_chunks_unchecked::() } + } + /// Looks up a key in this file. /// /// If `FIND_ALL` is false, returns after finding the first match. @@ -383,20 +537,12 @@ impl StaticSortedFile { key_block_cache: &BlockCache, value_block_cache: &BlockCache, ) -> Result { - // There is exactly one index block per file (always the last block). - // Read it first, then dispatch directly to the key block it points to. - let index_block_index = self.meta.block_count - 1; - let index_block = get_or_cache_block( - &self.backing, - &self.meta, - index_block_index, - key_block_cache, - &self.verified_blocks, - self.compression, - )?; - let key_block_index = self.lookup_index_block(&index_block, key_hash)?; + // The index block was resolved, verified and parsed at open time. + let key_block_index = self.lookup_index_block(key_hash); - let key_block_arc = get_or_cache_block( + // Borrowed, not owned: the search only reads the block, and any value it returns is + // either copied inline or points into a *value* block, so nothing outlives this call. + let key_block = get_or_read_block( &self.backing, &self.meta, key_block_index, @@ -404,48 +550,39 @@ impl StaticSortedFile { &self.verified_blocks, self.compression, )?; + let key_block = key_block.as_slice(); + let reader = ArcBlockCacheReader { backing: &self.backing, cache: value_block_cache, verified_blocks: &self.verified_blocks, }; - let block_type = be::read_u8(&key_block_arc); + let block_type = be::read_u8(key_block); match KeyBlockLayout::from_block_type(block_type) { - Some((layout, false)) => { - self.lookup_key_block::(key_block_arc, key_hash, key, layout, reader) + Some((layout, false)) => self + .lookup_variable_key_block::(key_block, key_hash, key, layout, reader), + Some((layout, true)) => { + self.lookup_fixed_key_block::(key_block, key_hash, key, layout, reader) } - Some((layout, true)) => self.lookup_fixed_key_block::( - key_block_arc, - key_hash, - key, - layout, - reader, - ), None => { bail!("Invalid block type"); } } } - /// Looks up a hash in a index block. - fn lookup_index_block(&self, block: &[u8], hash: u64) -> Result { - ensure!(block.len() >= 3, "index block too short"); - debug_assert!( - be::read_u8(block) == BLOCK_TYPE_INDEX, - "expected index block as last block" - ); - let first_block = be::read_u16(&block[1..]); - let (entries, remainder) = block[3..].as_chunks::(); - if entries.is_empty() { - return Ok(first_block); - } - if !remainder.is_empty() { - bail!("invalid index block, {} extra bytes", remainder.len()) - } + /// Finds the key block that would hold `hash`. + /// + /// Entry `i`'s hash is the lowest hash in the block it names, so a hash below the first entry + /// belongs to `first_block` and any other hash belongs to its predecessor entry's block. + /// Everything that does not depend on `hash` was resolved by [`IndexBlock::parse`] at open + /// time, so this is the binary search and nothing else. + #[inline] + fn lookup_index_block(&self, hash: u64) -> u16 { + let entries = self.index_entries(); match entries.binary_search_by(|entry| be::read_u64(entry).cmp(&hash)) { - Ok(i) => Ok(be::read_u16(&entries[i][8..])), - Err(0) => Ok(first_block), - Err(i) => Ok(be::read_u16(&entries[i - 1][8..])), + Ok(i) => be::read_u16(&entries[i][size_of::()..]), + Err(0) => self.index.first_block, + Err(i) => be::read_u16(&entries[i - 1][size_of::()..]), } } @@ -453,9 +590,9 @@ impl StaticSortedFile { /// /// If `FIND_ALL` is false, returns after finding the first match. /// If `FIND_ALL` is true, collects all entries with the same key. - fn lookup_key_block( + fn lookup_variable_key_block( &self, - block: ArcBytes, + block: &[u8], key_hash: u64, key: &K, layout: KeyBlockLayout, @@ -465,22 +602,17 @@ impl StaticSortedFile { ensure!(block.len() >= 4, "key block too short"); let entry_count = be::read_u24(&block[1..]) as usize; let data = &block[4..]; + let table_len = entry_count * key_block_table_stride(hash_len); ensure!( - data.len() >= entry_count * 4, + data.len() >= table_len, "key block too short for {entry_count} entries" ); - let offsets = &data[..entry_count * 4]; - let entries = &data[entry_count * 4..]; + let offsets = &data[..table_len]; + let entries = &data[table_len..]; - self.lookup_block_inner::( - &block, - entry_count, - key_hash, - key, - layout, - reader, - |i| get_key_entry(offsets, entries, entry_count, i, hash_len), - ) + self.lookup_block_inner::(entry_count, key_hash, key, layout, reader, |i| { + get_key_entry(offsets, entries, entry_count, i, hash_len) + }) } /// Looks up a key in a fixed-size key block. @@ -489,13 +621,12 @@ impl StaticSortedFile { /// enabling direct indexing during binary search. fn lookup_fixed_key_block( &self, - block: ArcBytes, + block: &[u8], key_hash: u64, key: &K, layout: KeyBlockLayout, reader: ArcBlockCacheReader<'_>, ) -> Result { - let hash_len = layout.hash_len(); ensure!(block.len() >= 6, "fixed key block too short"); let entry_count = be::read_u24(&block[1..]) as usize; let key_size = be::read_u8(&block[4..]) as usize; @@ -504,23 +635,17 @@ impl StaticSortedFile { value_type, val_size, header_size, - } = fixed_value_layout(&block, header_type)?; - let stride = hash_len as usize + key_size + val_size; + } = fixed_value_layout(block, header_type)?; + let regions = FixedRegions::new(entry_count, layout, key_size, val_size); let entries = &block[header_size..]; ensure!( - entries.len() == entry_count * stride, - "fixed key block for {entry_count} entries must is the wrong size" + entries.len() == regions.total_len(entry_count), + "fixed key block for {entry_count} entries is the wrong size" ); - self.lookup_block_inner::( - &block, - entry_count, - key_hash, - key, - layout, - reader, - |i| get_fixed_key_entry(entries, i, hash_len, key_size, value_type, stride), - ) + self.lookup_block_inner::(entry_count, key_hash, key, layout, reader, |i| { + get_fixed_key_entry(entries, i, regions, value_type) + }) } /// Shared binary search + collection logic for both key block variants. @@ -529,7 +654,6 @@ impl StaticSortedFile { /// key blocks (offset table lookup) and fixed-size key blocks (stride-based indexing). fn lookup_block_inner<'a, K: QueryKey, const FIND_ALL: bool>( &self, - block: &ArcBytes, entry_count: usize, key_hash: u64, key: &K, @@ -557,7 +681,7 @@ impl StaticSortedFile { if !FIND_ALL { // SingleValue mode: each key has exactly one entry // this is enforced when writing - let result = self.handle_key_match(ty, val, block, reader)?; + let result = self.handle_key_match(ty, val, reader)?; return Ok(SstLookupResult::Found(SmallVec::from_buf([result]))); } // FIND_ALL (MultiValue) mode: collect all values for this key. @@ -575,7 +699,7 @@ impl StaticSortedFile { if !entry_matches_key(layout, hash, entry_key, key_hash, key) { break; } - results.push(self.handle_key_match(ty, val, block, reader)?); + results.push(self.handle_key_match(ty, val, reader)?); } // Restore on-disk order: callers depend on both ends of the key group, with // key-value tombstones preceding the values they filter and a key tombstone @@ -583,7 +707,7 @@ impl StaticSortedFile { results.reverse(); // Add the entry at `m` - results.push(self.handle_key_match(ty, val, block, reader)?); + results.push(self.handle_key_match(ty, val, reader)?); for i in (m + 1)..r { let GetKeyEntryResult { hash, @@ -594,7 +718,7 @@ impl StaticSortedFile { if !entry_matches_key(layout, hash, entry_key, key_hash, key) { break; } - results.push(self.handle_key_match(ty, val, block, reader)?); + results.push(self.handle_key_match(ty, val, reader)?); } return Ok(SstLookupResult::Found(results)); } @@ -610,10 +734,51 @@ impl StaticSortedFile { &self, ty: u8, val: &[u8], - key_block_arc: &ArcBytes, reader: ArcBlockCacheReader<'_>, ) -> Result { - handle_key_match_generic(&self.meta, ty, val, key_block_arc, self.compression, reader) + handle_key_match_generic(&self.meta, ty, val, self.compression, reader) + } +} + +/// A block obtained from the backing store or the block cache. +/// +/// An uncompressed mmap block is borrowed straight out of the mmap. Only that borrow is needed to +/// search a key block, and taking it instead of an [`ArcBytes`] avoids touching the file's `mmap` +/// refcount — a single counter shared by every reader of the file, so the most contended one on +/// the read path. Anything that had to be decompressed or read into memory comes back owned, but +/// its refcount belongs to one cache entry rather than the whole file. +enum BlockRef<'l> { + /// Borrowed from the memory-mapped file. + Mmap(&'l [u8]), + /// Owned, and shared with the block cache. + Cached(ArcBytes), +} + +impl BlockRef<'_> { + #[inline] + fn as_slice(&self) -> &[u8] { + match self { + BlockRef::Mmap(data) => data, + BlockRef::Cached(block) => block, + } + } + + /// Promotes to an owned handle, taking a refcount for the mmap case. + /// + /// Only needed by callers that hand the bytes to something outliving the lookup. + #[inline] + fn into_owned(self, backing: &StaticSortedFileBacking) -> ArcBytes { + match self { + BlockRef::Mmap(data) => { + let StaticSortedFileBacking::Mmap(mmap) = backing else { + // `get_or_read_block` only borrows from an mmap backing. + unreachable!("mmap-borrowed block with a file backing") + }; + // SAFETY: the borrow came from this mmap, via `get_or_read_block`. + unsafe { ArcBytes::from_mmap(mmap, data) } + } + BlockRef::Cached(block) => block, + } } } @@ -621,18 +786,19 @@ impl StaticSortedFile { /// /// Reads the block header exactly once via `get_raw_block_slice` (which /// includes all `strict_checks` bounds guards). Uncompressed blocks bypass -/// the cache — an mmap-backed `ArcBytes` is cheaper than a cache lookup. -/// Their CRC is verified at most once per file open, tracked by -/// `verified_blocks`. Compressed blocks are looked up in `cache`; on a -/// miss they are decompressed, CRC-verified, and inserted. -fn get_or_cache_block( - backing: &StaticSortedFileBacking, +/// the cache and are borrowed from the mmap; their CRC is verified at most +/// once per file open, tracked by `verified_blocks`. Compressed blocks are +/// looked up in `cache`; on a miss they are decompressed, CRC-verified, and +/// inserted. File-backed blocks always go through the cache, including +/// uncompressed ones, since there is nothing to borrow from. +fn get_or_read_block<'l>( + backing: &'l StaticSortedFileBacking, meta: &StaticSortedFileMetaData, block_index: u16, cache: &BlockCache, verified_blocks: &[AtomicU64], compression: Compression, -) -> Result { +) -> Result> { let mmap_block = if let StaticSortedFileBacking::Mmap(mmap) = backing { let (uncompressed_length, checksum, block_data) = get_raw_block_slice(mmap, meta, block_index).with_context(|| { @@ -643,10 +809,10 @@ fn get_or_cache_block( })?; if uncompressed_length == 0 { - // Uncompressed: serve directly from mmap. Verify CRC only once per file open. + // Uncompressed: borrow directly from the mmap, taking no refcount. + // Verify CRC only once per file open. verify_checksum_once(meta, block_data, checksum, block_index, verified_blocks)?; - // SAFETY: block_data points into the mmap backing `mmap`. - return Ok(unsafe { ArcBytes::from_mmap(mmap, block_data) }); + return Ok(BlockRef::Mmap(block_data)); } Some((uncompressed_length, checksum, block_data)) } else { @@ -655,7 +821,7 @@ fn get_or_cache_block( // Compressed: check cache; decompress and insert on miss. // File-backed blocks use the same cache, including uncompressed ones. - Ok( + Ok(BlockRef::Cached( match cache.get_value_or_guard(&(meta.sequence_number, block_index), None) { GuardResult::Value(block) => block, GuardResult::Guard(guard) => { @@ -696,7 +862,7 @@ fn get_or_cache_block( } GuardResult::Timeout => unreachable!(), }, - ) + )) } /// Gets the raw block slice directly from a memory-mapped file. @@ -981,7 +1147,6 @@ fn handle_key_match_generic( meta: &StaticSortedFileMetaData, ty: u8, val: &[u8], - key_block: &B, compression: Compression, reader: impl ValueBlockCache, ) -> Result> { @@ -1007,15 +1172,12 @@ fn handle_key_match_generic( KEY_BLOCK_ENTRY_TYPE_KEY_DELETED => LookupValue::KeyDeleted, // Must precede the inline arm: both are open-ended and the tombstone range sits above it. ty if ty >= KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN => { - // The deleted value is stored inline, so `val` is already the correct slice. - // SAFETY: val points into key_block's data - let value = unsafe { key_block.slice_from_subslice(val) }; + let value = B::from_inline(val); LookupValue::KeyValueDeleted { value } } _ => { // Inline value — val is already the correct slice - // SAFETY: val points into key_block's data - let value = unsafe { key_block.slice_from_subslice(val) }; + let value = B::from_inline(val); LookupValue::Slice { value } } }) @@ -1056,17 +1218,19 @@ enum CurrentKeyBlockKind { Variable { offsets: RcBytes }, /// Fixed-size entries with uniform key size and value size (no offset table). Fixed { - key_size: usize, /// The type shared by every entry, or `None` if each entry carries its own type byte. value_type: Option, - stride: usize, + regions: FixedRegions, }, } impl CurrentKeyBlockKind { /// Decodes entry `index`, dispatching on the block's entry layout. + /// + /// The result borrows from `entries` and, for a variable block storing hashes, from the offset + /// table held by `self` — hence the shared lifetime. fn entry<'l>( - &self, + &'l self, entries: &'l [u8], entry_count: u32, index: usize, @@ -1077,10 +1241,9 @@ impl CurrentKeyBlockKind { get_key_entry(offsets, entries, entry_count as usize, index, hash_len) } CurrentKeyBlockKind::Fixed { - key_size, value_type, - stride, - } => get_fixed_key_entry(entries, index, hash_len, *key_size, *value_type, *stride), + regions, + } => get_fixed_key_entry(entries, index, *regions, *value_type), } } } @@ -1229,20 +1392,26 @@ impl StaticSortedFileIter { val_size, header_size, } = fixed_value_layout(data, data[5])?; - let stride = hash_len as usize + key_size + val_size; + let regions = FixedRegions::new(entry_count as usize, layout, key_size, val_size); let entries = block.slice(header_size..block_len); + ensure!( + entries.len() == regions.total_len(entry_count as usize), + "fixed key block for {entry_count} entries is the wrong size" + ); ( CurrentKeyBlockKind::Fixed { - key_size, value_type, - stride, + regions, }, entries, ) } else { let offset_table_begin = 4usize; - let offset_table_end = offset_table_begin + (entry_count as usize) * 4; - // In variable blocks the offsets table starts immediately after the entry count + let offset_table_end = 4 + (entry_count as usize) * key_block_table_stride(hash_len); + ensure!( + block_len >= offset_table_end, + "key block too short for {entry_count} entries" + ); let offsets = block.clone().slice(offset_table_begin..offset_table_end); let entries = block.slice(offset_table_end..block_len); (CurrentKeyBlockKind::Variable { offsets }, entries) @@ -1299,7 +1468,6 @@ impl StaticSortedFileIter { &self.meta, ty, val, - &kb.entries, self.compression, RcBlockCacheReader { backing: &self.backing, @@ -1401,6 +1569,11 @@ fn entry_matches_key( } /// Returns the byte size of the value portion for a given key block entry type. +/// +/// The type byte comes from the file, so the two open-ended ranges are bounded here rather than +/// trusted: the writer only ever emits sizes up to [`MAX_INLINE_VALUE_SIZE`], and a value that +/// large is what lets a lookup return it inline. Rejecting an over-large tag keeps that a total +/// function — `B::from_inline` would otherwise be handed more bytes than it can hold. fn entry_val_size(ty: u8) -> Result { match ty { KEY_BLOCK_ENTRY_TYPE_SMALL => Ok(SMALL_VALUE_REF_SIZE), @@ -1409,20 +1582,41 @@ fn entry_val_size(ty: u8) -> Result { KEY_BLOCK_ENTRY_TYPE_KEY_DELETED => Ok(KEY_DELETED_REF_SIZE), // Must precede the inline arm: both are open-ended and the tombstone range sits above it. ty if ty >= KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN => { - Ok((ty - KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN) as usize) + let size = (ty - KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN) as usize; + ensure!( + size <= MAX_INLINE_VALUE_SIZE, + "key-value tombstone type {ty} claims a {size} byte value, over the \ + {MAX_INLINE_VALUE_SIZE} byte maximum" + ); + Ok(size) } ty if ty >= KEY_BLOCK_ENTRY_TYPE_INLINE_MIN => { - Ok((ty - KEY_BLOCK_ENTRY_TYPE_INLINE_MIN) as usize) + let size = (ty - KEY_BLOCK_ENTRY_TYPE_INLINE_MIN) as usize; + ensure!( + size <= MAX_INLINE_VALUE_SIZE, + "inline value type {ty} claims a {size} byte value, over the \ + {MAX_INLINE_VALUE_SIZE} byte maximum" + ); + Ok(size) } _ => bail!("Invalid key block entry type: {ty}"), } } /// Reads the type and start offset from an offset table entry. -/// Each entry is 4 bytes: 1 byte type + 3 bytes BE offset. +/// +/// The trailing 4 bytes of every entry pack 1 byte of type into the top of a 3-byte BE offset. +/// `HashThenKey` entries carry the key's 8-byte hash ahead of that word — see +/// [`KEY_BLOCK_TABLE_ENTRY_SIZE_WITH_HASH`]. #[inline(always)] -fn read_offset_entry(offsets: &[u8], index: usize) -> (u8, usize) { - let base = index * 4; +fn read_offset_entry( + offsets: &[u8], + index: usize, + table_stride: usize, + hash_len: u8, +) -> (u8, usize) { + // The offset word is last, so skip any hash that precedes it. + let base = index * table_stride + (hash_len as usize); let word = be::read_u32(&offsets[base..]); let ty = (word >> 24) as u8; let offset = (word & 0x00FF_FFFF) as usize; @@ -1431,26 +1625,26 @@ fn read_offset_entry(offsets: &[u8], index: usize) -> (u8, usize) { /// Reads a key entry from a key block. fn get_key_entry<'l>( - offsets: &[u8], + offsets: &'l [u8], entries: &'l [u8], entry_count: usize, index: usize, hash_len: u8, ) -> Result> { - let hash_len_usize = hash_len as usize; - let (ty, start) = read_offset_entry(offsets, index); + let table_stride = key_block_table_stride(hash_len); + let (ty, start) = read_offset_entry(offsets, index, table_stride, hash_len); let end = if index == entry_count - 1 { entries.len() } else { - let (_, next_start) = read_offset_entry(offsets, index + 1); + let (_, next_start) = read_offset_entry(offsets, index + 1, table_stride, hash_len); next_start }; - // Return the raw hash bytes slice (0-8 bytes depending on hash_len) - let hash = &entries[start..start + hash_len_usize]; + // Hoisted into the table, so the search never reaches into the payload; empty for `KeyOnly`. + let hash = &offsets[index * table_stride..index * table_stride + hash_len as usize]; let val_size = entry_val_size(ty)?; Ok(GetKeyEntryResult { hash, - key: &entries[start + hash_len_usize..end - val_size], + key: &entries[start..end - val_size], ty, val: &entries[end - val_size..end], }) @@ -1476,10 +1670,17 @@ fn fixed_value_layout(block: &[u8], header_type: u8) -> Result // Mixed-type block: the value size follows the header's type byte, and each entry // carries its own type. ensure!(block.len() >= 7, "mixed-type fixed key block too short"); + // Validate the value footprint byte + let value_footprint = be::read_u8(&block[6..]) as usize; + ensure!( + value_footprint <= MAX_INLINE_VALUE_SIZE, + "mixed-type fixed key block claims a {value_footprint} byte value footprint, over the \ + {MAX_INLINE_VALUE_SIZE} byte maximum" + ); Ok(FixedValueLayout { value_type: None, // +1 for the per-entry type byte, which is part of the stride. - val_size: be::read_u8(&block[6..]) as usize + 1, + val_size: value_footprint + 1, header_size: 7, }) } else { @@ -1491,28 +1692,104 @@ fn fixed_value_layout(block: &[u8], header_type: u8) -> Result } } +/// Where the two regions of a fixed-size key block sit, computed once per block. +/// +/// A fixed block stores the bytes the binary search probes in a dense leading region and everything +/// else in a trailing region at the same entry index, so a probe touches one small stride rather +/// than a full interleaved entry. +/// +/// This is the single owner of that geometry: the reader, the writer, and `sst_inspect` all derive +/// their offsets from here, so a change to which bytes go in the search region is made once. It is +/// the fixed-block counterpart to [`key_block_table_stride`] for variable-size blocks. +#[derive(Clone, Copy)] +pub struct FixedRegions { + /// Which bytes the search region holds: the hash (`HashThenKey`) or the key (`KeyOnly`). + layout: KeyBlockLayout, + /// Bytes per entry in the search region. + pub search_stride: usize, + /// Offset of the tail region, relative to the start of the entry data. + pub tail_start: usize, + /// Bytes per entry in the tail region. + pub tail_stride: usize, + key_size: usize, +} + +impl FixedRegions { + /// `val_size` is the tail's per-entry value footprint: the value bytes plus the per-entry type + /// byte of a mixed-type block. [`fixed_value_layout`] already folds that byte in; a caller + /// computing it from a block header must add it itself. + pub fn new( + entry_count: usize, + layout: KeyBlockLayout, + key_size: usize, + val_size: usize, + ) -> Self { + // `HashThenKey` searches the hashes and keeps the key with the value; `KeyOnly` has no + // hash, so the key itself is the search region. + let (search_stride, tail_stride) = match layout { + KeyBlockLayout::HashThenKey => (layout.hash_len() as usize, key_size + val_size), + KeyBlockLayout::KeyOnly => (key_size, val_size), + }; + Self { + layout, + search_stride, + tail_start: entry_count * search_stride, + tail_stride, + key_size, + } + } + + /// Bytes of a tail entry that precede its value: the key for `HashThenKey`, nothing for + /// `KeyOnly`, which keeps its key in the search region. + pub fn tail_key_size(&self) -> usize { + match self.layout { + KeyBlockLayout::HashThenKey => self.key_size, + KeyBlockLayout::KeyOnly => 0, + } + } + + /// Total entry-data length implied by these regions, for bounds checking. + pub fn total_len(&self, entry_count: usize) -> usize { + self.tail_start + entry_count * self.tail_stride + } +} + fn get_fixed_key_entry<'l>( entries: &'l [u8], index: usize, - hash_len: u8, - key_size: usize, + regions: FixedRegions, value_type: Option, - stride: usize, ) -> Result> { - let hash_len_usize = hash_len as usize; - let start = index * stride; - let key_start = start + hash_len_usize; - let key_end = key_start + key_size; - // In a mixed-type block the entry's type byte sits between its key and its value. + let FixedRegions { + layout, + search_stride, + tail_start, + tail_stride, + key_size, + } = regions; + // The search region holds only what the binary search compares first: the hash for + // `HashThenKey` blocks, the key for `KeyOnly` blocks. Everything else lives in the tail region + // at the same entry index. + let search = index * search_stride; + let tail = tail_start + index * tail_stride; + let (hash, key, tail_rest) = match layout { + KeyBlockLayout::HashThenKey => ( + &entries[search..search + search_stride], + &entries[tail..tail + key_size], + tail + key_size, + ), + KeyBlockLayout::KeyOnly => (&entries[..0], &entries[search..search + key_size], tail), + }; + // In a mixed-type block the entry's type byte precedes its value in the tail region. let (ty, val_start) = match value_type { - Some(ty) => (ty, key_end), - None => (be::read_u8(&entries[key_end..]), key_end + 1), + Some(ty) => (ty, tail_rest), + None => (be::read_u8(&entries[tail_rest..]), tail_rest + 1), }; Ok(GetKeyEntryResult { - hash: &entries[start..key_start], - key: &entries[key_start..key_end], + hash, + key, ty, - val: &entries[val_start..(index + 1) * stride], + val: &entries[val_start..tail + tail_stride], }) } diff --git a/turbopack/crates/turbo-persistence/src/static_sorted_file_builder.rs b/turbopack/crates/turbo-persistence/src/static_sorted_file_builder.rs index c112788ed0f5..1be635917667 100644 --- a/turbopack/crates/turbo-persistence/src/static_sorted_file_builder.rs +++ b/turbopack/crates/turbo-persistence/src/static_sorted_file_builder.rs @@ -7,6 +7,7 @@ use std::{ use anyhow::{Context, Result}; use byteorder::{BE, ByteOrder, WriteBytesExt}; +use either::Either; use fs_err::File; use crate::{ @@ -15,11 +16,12 @@ use crate::{ constants::{MAX_INLINE_VALUE_SIZE, MAX_SMALL_VALUE_SIZE, MIN_SMALL_VALUE_BLOCK_SIZE}, meta_file::MetaEntryFlags, static_sorted_file::{ - BLOB_VALUE_REF_SIZE, BLOCK_TYPE_INDEX, FIXED_KEY_BLOCK_MIXED_VALUE_TYPE, + BLOB_VALUE_REF_SIZE, BLOCK_TYPE_INDEX, FIXED_KEY_BLOCK_MIXED_VALUE_TYPE, FixedRegions, KEY_BLOCK_ENTRY_TYPE_BLOB, KEY_BLOCK_ENTRY_TYPE_INLINE_MIN, KEY_BLOCK_ENTRY_TYPE_KEY_DELETED, KEY_BLOCK_ENTRY_TYPE_KEY_VALUE_DELETED_MIN, - KEY_BLOCK_ENTRY_TYPE_MEDIUM, KEY_BLOCK_ENTRY_TYPE_SMALL, KEY_DELETED_REF_SIZE, - KeyBlockLayout, MEDIUM_VALUE_REF_SIZE, SMALL_VALUE_REF_SIZE, + KEY_BLOCK_ENTRY_TYPE_MEDIUM, KEY_BLOCK_ENTRY_TYPE_SMALL, + KEY_BLOCK_TABLE_ENTRY_SIZE_NO_HASH, KEY_DELETED_REF_SIZE, KeyBlockLayout, + MEDIUM_VALUE_REF_SIZE, SMALL_VALUE_REF_SIZE, key_block_table_stride, }, }; @@ -332,6 +334,9 @@ pub struct StaticSortedFileBuilderMeta<'a> { /// Writes an SST file from a pre-sorted slice of entries. /// +/// Entries must be sorted in (key-hash, key) order, the same contract as +/// [`StreamingSstWriter::add`]. +/// /// This is a convenience wrapper around [`StreamingSstWriter`] for callers that already have all /// entries in memory. // TODO: Consider adding a variant that takes ownership (Vec or drain iterator) @@ -342,7 +347,12 @@ pub fn write_static_stored_file( flags: MetaEntryFlags, compression: Compression, ) -> Result<(StaticSortedFileBuilderMeta<'static>, File)> { - debug_assert!(entries.iter().map(|e| e.key_hash()).is_sorted()); + debug_assert!( + entries + .iter() + .map(|e| (e.key_hash(), e.key_bytes())) + .is_sorted() + ); let mut writer = StreamingSstWriter::new(file, flags, entries.len() as u64, compression)?; for entry in entries { writer.add(entry)?; @@ -580,6 +590,10 @@ pub struct StreamingSstWriter { // Reusable buffer for building key blocks key_buffer: Vec, + // Reusable buffer for the tail region of a key block: the key (when the search region holds + // hashes) and the value. Appended to `key_buffer` when the block is finished. + key_value_buffer: Vec, + // Collected key hashes truncated to u32 for deferred AMQF construction via sorted Builder // in close(). Fingerprint size is always <32 bits, so the lower 32 bits suffice. collected_fingerprints: Vec, @@ -646,7 +660,11 @@ impl StreamingSstWriter { pending_small_value_block: Vec::with_capacity( MIN_SMALL_VALUE_BLOCK_SIZE + MAX_SMALL_VALUE_SIZE, ), + // `FixedKeyBlockBuilder::finish` appends the tail back into `key_buffer`, so it still + // holds a whole block. The tail buffer is only used by fixed-size blocks and is + // `reserve`d to the exact region size per block, so it starts empty. key_buffer: Vec::with_capacity(MAX_KEY_BLOCK_SIZE), + key_value_buffer: Vec::new(), collected_fingerprints: Vec::with_capacity(max_entry_count as usize), key_block_boundaries: Vec::with_capacity(estimated_key_blocks), min_hash: u64::MAX, @@ -941,20 +959,28 @@ impl StreamingSstWriter { // loops read `pending_keys`. let Self { key_buffer, + key_value_buffer, pending_keys, .. } = self; key_buffer.clear(); - let build_key_order = |start: usize, end: usize| -> Vec<&PendingEntry> { - let mut key_order: Vec<&PendingEntry> = pending_keys.range(start..end).collect(); - + // The layout fixes the order entries must be written in, the same way it fixes their + // encoding, so deriving the order from the same `layout` the builders encode by keeps the + // two from disagreeing. `KeyOnly` blocks are searched by key and need a re-sorted copy; + // `HashThenKey` blocks are already in the caller's `(hash, key)` order and are yielded + // straight from `pending_keys` with no allocation. + let block_entries = |start: usize, end: usize| { + if layout == KeyBlockLayout::HashThenKey { + return Either::Left(pending_keys.range(start..end)); + } + let mut entries: Vec<&PendingEntry> = pending_keys.range(start..end).collect(); // Stable sort is important to preserve relative order of tombstones match info.uniform_key_len() { - Some(4) => key_order.sort_by_key(|&e| be_key_u32(e.entry.key_bytes())), - Some(8) => key_order.sort_by_key(|&e| be_key_u64(e.entry.key_bytes())), - _ => key_order.sort_by_key(|&e| e.entry.key_bytes()), + Some(4) => entries.sort_by_key(|&e| be_key_u32(e.entry.key_bytes())), + Some(8) => entries.sort_by_key(|&e| be_key_u64(e.entry.key_bytes())), + _ => entries.sort_by_key(|&e| e.entry.key_bytes()), } - key_order + Either::Right(entries.into_iter()) }; if let KeyBlockFormat::Fixed { @@ -965,34 +991,22 @@ impl StreamingSstWriter { { let mut builder = FixedKeyBlockBuilder::new( key_buffer, + key_value_buffer, entry_count as u32, layout, key_size, val_size, value_type, ); - if layout == KeyBlockLayout::KeyOnly { - for pending in build_key_order(start, end) { - builder.put(&pending.entry, &pending.value_ref); - } - } else { - for pending in pending_keys.range(start..end) { - builder.put_with_hash(&pending.entry, &pending.value_ref); - } + for pending in block_entries(start, end) { + builder.put(&pending.entry, &pending.value_ref); } builder.finish(); } else { let mut builder = KeyBlockBuilder::new(key_buffer, entry_count as u32, layout); - if layout == KeyBlockLayout::KeyOnly { - for pending in build_key_order(start, end) { - builder.put(&pending.entry, &pending.value_ref); - } - } else { - for pending in pending_keys.range(start..end) { - builder.put_with_hash(&pending.entry, &pending.value_ref); - } + for pending in block_entries(start, end) { + builder.put(&pending.entry, &pending.value_ref); } - builder.finish(); } @@ -1182,11 +1196,16 @@ impl Drop for StreamingSstWriter { /// Builder for a single key block. /// -/// Entries are added via `put_*` methods which write key data and value references into the buffer. +/// Entries are added via [`Self::put`], which writes key data and value references into the buffer. /// The block format uses a fixed-size header table followed by variable-length entry data. struct KeyBlockBuilder<'l> { current_entry: usize, header_size: usize, + /// Whether entries hoist their hash into the table slot. Chosen at construction and consulted + /// by [`Self::put`], so a caller cannot pair a block with the wrong entry encoding. + layout: KeyBlockLayout, + /// Bytes per offset table entry, which is wider when the block stores hashes. + table_stride: usize, buffer: &'l mut Vec, } @@ -1199,40 +1218,41 @@ impl<'l> KeyBlockBuilder<'l> { debug_assert!(entry_count < (1 << 24)); const ESTIMATED_KEY_SIZE: usize = 16; - buffer.reserve(entry_count as usize * ESTIMATED_KEY_SIZE); + let table_stride = key_block_table_stride(layout.hash_len()); + buffer.reserve(entry_count as usize * (ESTIMATED_KEY_SIZE + table_stride)); let block_type = layout.block_type(false); buffer.write_u8(block_type).unwrap(); buffer.write_u24::(entry_count).unwrap(); - for _ in 0..entry_count { - buffer.write_u32::(0).unwrap(); - } + // Reserve the offset table; each entry's slot is filled in as it is written. + buffer.resize(buffer.len() + entry_count as usize * table_stride, 0); Self { current_entry: 0, header_size: buffer.len(), + layout, + table_stride, buffer, } } - /// Writes the entry header (position + type) for the current entry. + /// Writes the type and payload position into the current entry's table slot. + /// + /// The word sits at the end of the slot, after the hash for a `HashThenKey` block. fn write_entry_header(&mut self, entry_type: EntryType) { let pos = self.buffer.len() - self.header_size; - let header_offset = KEY_BLOCK_HEADER_SIZE + self.current_entry * 4; + let slot = KEY_BLOCK_HEADER_SIZE + self.current_entry * self.table_stride; + let word_offset = slot + self.table_stride - KEY_BLOCK_TABLE_ENTRY_SIZE_NO_HASH; let header = (pos as u32) | ((entry_type.0 as u32) << 24); - BE::write_u32(&mut self.buffer[header_offset..header_offset + 4], header); + BE::write_u32(&mut self.buffer[word_offset..word_offset + 4], header); } - /// Writes a single entry (header + key + value data) to the block. + /// Writes a single entry (table slot + maybe hash? + key + value data) to the block. fn put(&mut self, entry: &E, value_ref: &ValueRef) { self.write_entry_header(value_ref.entry_type()); - entry.write_key_to(self.buffer); - value_ref.write_value_to(self.buffer); - self.current_entry += 1; - } - /// Writes a single entry (header + hash + key + value data) to the block. - fn put_with_hash(&mut self, entry: &E, value_ref: &ValueRef) { - self.write_entry_header(value_ref.entry_type()); - self.buffer - .extend_from_slice(&entry.key_hash().to_be_bytes()); + if self.layout == KeyBlockLayout::HashThenKey { + let slot = KEY_BLOCK_HEADER_SIZE + self.current_entry * self.table_stride; + self.buffer[slot..slot + size_of::()] + .copy_from_slice(&entry.key_hash().to_be_bytes()); + } entry.write_key_to(self.buffer); value_ref.write_value_to(self.buffer); self.current_entry += 1; @@ -1256,25 +1276,57 @@ const FIXED_KEY_BLOCK_HEADER_SIZE: usize = 6; /// No offset table is written — entry positions are computed arithmetically from the stride. When /// entries share a value size but not a value type, the header records /// [`FIXED_KEY_BLOCK_MIXED_VALUE_TYPE`] and each entry carries its own type byte before its value. +/// +/// Entries are written as two regions rather than interleaved, so that the bytes a lookup's binary +/// search probes are contiguous: the search region holds only what the lookup compares first (the +/// hash for `HashThenKey`, the key for `KeyOnly`), and everything else follows in the tail region, +/// addressed by the same entry index. [`FixedRegions`] derives that geometry for both this builder +/// and the reader; see [`KEY_BLOCK_TABLE_ENTRY_SIZE_WITH_HASH`] for why the compared bytes are +/// hoisted out of the payload. struct FixedKeyBlockBuilder<'l> { + /// Receives the header and then the search region. buffer: &'l mut Vec, + /// Accumulates the tail region, appended to `buffer` by [`Self::finish`]. + tail: &'l mut Vec, /// Whether each entry writes its own type byte (set for mixed-type blocks). per_entry_type: bool, + /// Which of the two regions the key goes in: the search region for `KeyOnly`, the tail for + /// `HashThenKey`. Also checks that callers pair the layout with the matching `put` method. + layout: KeyBlockLayout, } impl<'l> FixedKeyBlockBuilder<'l> { fn new( buffer: &'l mut Vec, + tail: &'l mut Vec, entry_count: u32, layout: KeyBlockLayout, key_size: u8, val_size: u8, value_type: Option, ) -> Self { - let hash_len = layout.hash_len() as usize; let per_entry_type = value_type.is_none(); - let stride = hash_len + key_size as usize + val_size as usize + usize::from(per_entry_type); - buffer.reserve(FIXED_KEY_BLOCK_HEADER_SIZE + entry_count as usize * stride); + // The two regions partition the entry bytes: the search region takes the bytes compared + // first, the tail takes the rest. `FixedRegions` owns that split for reader and writer + // alike, so the geometry is derived in one place. Its `val_size` includes the per-entry + // type byte, which the block header keeps separate from the value size. + let FixedRegions { + search_stride, + tail_stride, + .. + } = FixedRegions::new( + entry_count as usize, + layout, + key_size as usize, + val_size as usize + usize::from(per_entry_type), + ); + // `finish` appends the tail back into `buffer`, so reserve room for the whole block here + // and the append never reallocates. + buffer.reserve( + FIXED_KEY_BLOCK_HEADER_SIZE + entry_count as usize * (search_stride + tail_stride), + ); + tail.clear(); + tail.reserve(entry_count as usize * tail_stride); let block_type = layout.block_type(true); buffer.extend_from_slice(&[ @@ -1293,30 +1345,37 @@ impl<'l> FixedKeyBlockBuilder<'l> { Self { buffer, + tail, per_entry_type, + layout, } } - /// Writes a single entry (key + optional type byte + value data) to the block. + /// Writes a single entry, splitting it between the two regions according to the block's + /// layout: `HashThenKey` puts the hash in the search region and the key in the tail, `KeyOnly` + /// puts the key itself in the search region. The layout decides that, not the caller. fn put(&mut self, entry: &E, value_ref: &ValueRef) { - entry.write_key_to(self.buffer); - if self.per_entry_type { - self.buffer.push(value_ref.entry_type().0); + match self.layout { + KeyBlockLayout::HashThenKey => { + self.buffer + .extend_from_slice(&entry.key_hash().to_be_bytes()); + entry.write_key_to(self.tail); + } + KeyBlockLayout::KeyOnly => entry.write_key_to(self.buffer), } - value_ref.write_value_to(self.buffer); + self.put_tail(value_ref); } - /// Writes a single entry (hash + key + optional type byte + value data) to the block. - fn put_with_hash(&mut self, entry: &E, value_ref: &ValueRef) { - self.buffer - .extend_from_slice(&entry.key_hash().to_be_bytes()); - entry.write_key_to(self.buffer); + /// Appends the parts of an entry that the search never reads. + fn put_tail(&mut self, value_ref: &ValueRef) { if self.per_entry_type { - self.buffer.push(value_ref.entry_type().0); + self.tail.push(value_ref.entry_type().0); } - value_ref.write_value_to(self.buffer); + value_ref.write_value_to(self.tail); } + fn finish(self) -> &'l mut Vec { + self.buffer.extend_from_slice(self.tail); self.buffer } }