[pull] canary from vercel:canary

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,17 +210,32 @@ 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)
 
 The mixed-type form exists so that same-sized inline values and key-value tombstones can share a
 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
 

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();

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<Mmap> },
+enum Repr {
+    Arc {
+        data: *const [u8],
+        _backing: Arc<[u8]>,
+    },
+    Mmap {
+        data: *const [u8],
+        _backing: Arc<Mmap>,
+    },
+    /// 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<Arc<[u8]>> 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,17 +114,17 @@ 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
     /// are other `Arc` clones referencing the same data outside the cache).
     /// 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<Mmap>;
 
     fn slice(self, range: Range<usize>) -> 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,
+            },
+        }
+    }
 }