Skip to content

Commit fdfe9b4

Browse files
committed
perf(btree): make the reused-page sink a set so the free-time check is O(1)
The free-time eligibility test introduced with `freed_reusable` still scanned `free_page_consumed` linearly, and a copy-on-write free of a pre-existing page is below the reuse threshold by construction — so the dominant case in a flush kept paying O(frees x |consumed|), with |consumed| bounded by the free-list window rather than by anything small. Making the shared sink an FxHashSet drops that to O(1) per free: the commit path already collapsed it into a set and only tests membership, begin_write only clears it, and nothing reads its order. The hang detector now seeds `consumed` at window scale (WINDOW_PAGES x chain_capacity(page_size)) instead of a token 500 entries; at the old size it could not observe this term at all. set_reuse_threshold now debug_asserts that nothing has been freed yet, as do the two sibling setters, since eligibility is decided at free time and a threshold arriving afterwards would judge already-classified pages against the wrong one. That makes the previous re-partition branch unreachable, so it and the test covering it are gone rather than left as dead code. Also drops the `reuse_threshold == 0` disjunct, which never decided anything: every page id is >= 0.
1 parent 7487e5f commit fdfe9b4

6 files changed

Lines changed: 60 additions & 54 deletions

File tree

CHANGELOG.md

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,12 +2,6 @@
22

33
All notable changes to this project are documented here. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
44

5-
## [Unreleased]
6-
7-
### Fixed
8-
9-
- **Page allocation no longer rescans the freed list.** `allocate_page` decided in-session reuse eligibility per call, scanning every page freed so far and, for each, every page drawn from the shared free-page cache — both `Vec<u64>`, so the membership test was linear. Since the reuse threshold is the session-start `next_page_id`, every page freed during a transaction fell below it and took the slow arm, making allocation quadratic in the pages a flush touches: a flush large enough would occupy a core indefinitely without completing. Eligibility is now decided once, when the page is freed, and allocation is a pop. Behaviour is unchanged — both inputs to the decision are monotonic within a session, so deciding early is equivalent.
10-
115
## [0.1.0] - 2026-07-28
126

137
The first release. Pre-releases were published as `0.1.0-beta.N`; the entries below describe `0.1.0` as a whole rather than deltas against a shipped version, since none exists yet.

src/btree/tree/core.rs

Lines changed: 56 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
use std::collections::BTreeSet;
44
use std::sync::Arc;
55

6-
use rustc_hash::FxHashMap;
6+
use rustc_hash::{FxHashMap, FxHashSet};
77

88
use crate::errors::PagedbError;
99
use crate::pager::format::page_kind::PageKind;
@@ -168,7 +168,13 @@ pub struct BTree<V: Vfs> {
168168
/// Sink recording page ids drawn from `free_page_cache` and reused this
169169
/// session. The commit path removes them from the durable free-list (they
170170
/// now hold live committed data). Shared (via `Arc`) across the txn's trees.
171-
pub(super) free_page_consumed: Option<Arc<parking_lot::Mutex<Vec<u64>>>>,
171+
///
172+
/// A set, not a list: [`Self::free_page`] tests membership once per freed
173+
/// page, and the size is bounded by the free-list window
174+
/// (`WINDOW_PAGES × chain_capacity(page_size)`) rather than by anything
175+
/// small — a linear test here would keep the per-free cost proportional to
176+
/// the window. Nothing reads the insertion order or needs duplicates.
177+
pub(super) free_page_consumed: Option<Arc<parking_lot::Mutex<FxHashSet<u64>>>>,
172178
/// Last key successfully appended via [`Self::put_append`]. Used to
173179
/// enforce the monotonic-key invariant on subsequent calls and to
174180
/// invalidate the cached path when any non-append mutation (regular
@@ -218,20 +224,16 @@ impl<V: Vfs> BTree<V> {
218224
/// pages below it are still referenced by the last durable header and must
219225
/// keep their on-disk bytes until the header that frees them is durable.
220226
///
221-
/// Re-partitions anything already freed, since eligibility is decided at
222-
/// free time: callers normally set the threshold before the first free, so
223-
/// this is a no-op walk over two empty vectors.
227+
/// Must be called before the first free: [`Self::free_page`] decides reuse
228+
/// eligibility at the moment of the free, so a threshold arriving later
229+
/// would leave already-classified pages judged against the wrong one. Every
230+
/// caller sets it on a fresh tree, which the `debug_assert` states.
224231
pub fn set_reuse_threshold(&mut self, threshold: u64) {
232+
debug_assert!(
233+
self.freed.is_empty() && self.freed_reusable.is_empty(),
234+
"set_reuse_threshold after a free: eligibility is decided at free time"
235+
);
225236
self.reuse_threshold = threshold;
226-
let mut pending = std::mem::take(&mut self.freed);
227-
pending.append(&mut self.freed_reusable);
228-
for id in pending {
229-
if self.is_reusable_in_session(id) {
230-
self.freed_reusable.push(id);
231-
} else {
232-
self.freed.push(id);
233-
}
234-
}
235237
}
236238

237239
/// Wire in the `Db`'s shared free-page cache. After this call,
@@ -245,13 +247,21 @@ impl<V: Vfs> BTree<V> {
245247
/// pushed in from elsewhere would be handed out while the unscanned tail
246248
/// still named it — the same page id given to two live structures.
247249
pub fn set_free_page_cache(&mut self, cache: Arc<parking_lot::Mutex<Vec<u64>>>) {
250+
debug_assert!(
251+
self.freed.is_empty() && self.freed_reusable.is_empty(),
252+
"set_free_page_cache after a free: eligibility is decided at free time"
253+
);
248254
self.free_page_cache = Some(cache);
249255
}
250256

251257
/// Wire the shared sink that records cache pages reused this session, so the
252258
/// commit path can remove them from the durable free-list. Set alongside
253259
/// [`Self::set_free_page_cache`].
254-
pub fn set_free_page_consumed(&mut self, consumed: Arc<parking_lot::Mutex<Vec<u64>>>) {
260+
pub fn set_free_page_consumed(&mut self, consumed: Arc<parking_lot::Mutex<FxHashSet<u64>>>) {
261+
debug_assert!(
262+
self.freed.is_empty() && self.freed_reusable.is_empty(),
263+
"set_free_page_consumed after a free: eligibility is decided at free time"
264+
);
255265
self.free_page_consumed = Some(consumed);
256266
}
257267

@@ -310,7 +320,7 @@ impl<V: Vfs> BTree<V> {
310320
"allocate_page recycled reserved page {id} from free-list cache"
311321
);
312322
if let Some(consumed) = &self.free_page_consumed {
313-
consumed.lock().push(id);
323+
consumed.lock().insert(id);
314324
}
315325
return id;
316326
}
@@ -346,7 +356,8 @@ impl<V: Vfs> BTree<V> {
346356
/// cache this session, in which case the durable header references none of
347357
/// its content.
348358
fn is_reusable_in_session(&self, page_id: u64) -> bool {
349-
if self.reuse_threshold == 0 || page_id >= self.reuse_threshold {
359+
// A zero threshold needs no special case: every page id clears it.
360+
if page_id >= self.reuse_threshold {
350361
return true;
351362
}
352363
self.free_page_consumed
@@ -499,7 +510,7 @@ impl<V: Vfs> BTree<V> {
499510
mod tests {
500511
use std::sync::Arc;
501512

502-
use super::BTree;
513+
use super::{BTree, FxHashSet};
503514
use crate::RealmId;
504515
use crate::crypto::CipherId;
505516
use crate::crypto::kdf::derive_mk;
@@ -534,7 +545,7 @@ mod tests {
534545
#[tokio::test(flavor = "current_thread")]
535546
async fn reuse_eligibility_matches_the_threshold_rule() {
536547
let mut tree = fresh_tree().await;
537-
let consumed = Arc::new(parking_lot::Mutex::new(Vec::new()));
548+
let consumed = Arc::new(parking_lot::Mutex::new(FxHashSet::default()));
538549
tree.set_free_page_consumed(Arc::clone(&consumed));
539550
tree.set_reuse_threshold(1000);
540551

@@ -547,46 +558,49 @@ mod tests {
547558
assert_eq!(tree.allocate_page(), 1500);
548559

549560
// Below threshold but drawn from the shared cache this session.
550-
consumed.lock().push(600);
561+
consumed.lock().insert(600);
551562
tree.free_page(600);
552563
assert_eq!(tree.allocate_page(), 600);
553564

554565
// The one page held back is still reported as freed this session.
555566
assert_eq!(tree.drain_freed(), vec![500]);
556567
}
557568

558-
/// A page freed before the threshold was set is classified when it is set.
559-
#[tokio::test(flavor = "current_thread")]
560-
async fn threshold_set_after_a_free_repartitions() {
561-
let mut tree = fresh_tree().await;
562-
tree.free_page(500);
563-
tree.free_page(1500);
564-
tree.set_reuse_threshold(1000);
565-
assert_eq!(tree.allocate_page(), 1500, "above threshold stays reusable");
566-
assert_eq!(tree.allocate_page(), 4, "below threshold was held back");
567-
assert_eq!(tree.drain_freed(), vec![500]);
568-
}
569-
570-
/// Allocation must not rescan the freed list. A hang detector, not a
571-
/// benchmark: the bound is ~1000x the fixed cost, and only a return to a
572-
/// per-allocation scan over `freed` x `free_page_consumed` can exceed it.
573-
/// That scan is what pinned a core for 84 minutes inside one flush.
569+
/// Neither freeing nor allocating may scan a collection that grows with the
570+
/// flush. A hang detector, not a benchmark — the only wall-clock assertion
571+
/// in `src/`, and the bound is ~1000x the fixed cost, so only a return to a
572+
/// linear scan can exceed it. Two such scans are in scope: allocation over
573+
/// `freed`, and the free-time eligibility test over `free_page_consumed`.
574+
///
575+
/// `consumed` is therefore seeded at the size the free-list window permits
576+
/// (`WINDOW_PAGES × chain_capacity(page_size)`), not at a token count: that
577+
/// is what bounds it in a real flush, and a smaller seed cannot see the
578+
/// free-time term at all.
574579
#[tokio::test(flavor = "current_thread")]
575580
async fn allocation_cost_does_not_grow_with_the_freed_list() {
576581
const N: u64 = 20_000;
582+
let window = crate::pager::freelist::WINDOW_PAGES
583+
* crate::pager::freelist::layout::chain_capacity(PAGE);
577584
let mut tree = fresh_tree().await;
578-
let consumed = Arc::new(parking_lot::Mutex::new(Vec::new()));
585+
let consumed = Arc::new(parking_lot::Mutex::new(FxHashSet::default()));
579586
tree.set_free_page_consumed(Arc::clone(&consumed));
580587
tree.set_reuse_threshold(N * 2);
581588

582-
// Every one of these is ineligible, so each allocation below had to
583-
// scan all of them (and, per entry, all of `consumed`) before the fix.
589+
// Seed the sink at window scale *before* the frees, so every free below
590+
// pays the eligibility test against a full-size `consumed`.
591+
{
592+
let mut c = consumed.lock();
593+
for id in (N * 4)..(N * 4 + window as u64) {
594+
c.insert(id);
595+
}
596+
}
597+
598+
// Every one of these is below the threshold and absent from `consumed`,
599+
// so it is ineligible: before the fix each allocation scanned all of
600+
// them and, per entry, all of `consumed`.
584601
for id in 4..N {
585602
tree.free_page(id);
586603
}
587-
for id in N..(N + 500) {
588-
consumed.lock().push(id);
589-
}
590604

591605
let start = std::time::Instant::now();
592606
for _ in 0..N {

src/txn/db/core.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -234,7 +234,7 @@ pub struct Db<V: Vfs + Clone> {
234234
/// Per-txn sink (cleared at `begin_write`) recording page ids the allocator
235235
/// drew from `free_page_cache`. The commit path removes them from the
236236
/// durable free-list — they now hold live committed data.
237-
pub(crate) free_page_consumed: Arc<parking_lot::Mutex<Vec<u64>>>,
237+
pub(crate) free_page_consumed: Arc<parking_lot::Mutex<rustc_hash::FxHashSet<u64>>>,
238238
#[cfg(test)]
239239
pub(crate) visibility_test_hook: parking_lot::Mutex<Option<Arc<VisibilityTestHook>>>,
240240
#[cfg(test)]

src/txn/db/open/create.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ impl<V: Vfs + Clone> Db<V> {
130130
})),
131131
poisoned_commit: parking_lot::Mutex::new(None),
132132
free_page_cache: Arc::new(parking_lot::Mutex::new(Vec::new())),
133-
free_page_consumed: Arc::new(parking_lot::Mutex::new(Vec::new())),
133+
free_page_consumed: Arc::new(parking_lot::Mutex::new(rustc_hash::FxHashSet::default())),
134134
#[cfg(test)]
135135
visibility_test_hook: parking_lot::Mutex::new(None),
136136
#[cfg(test)]

src/txn/db/open/existing.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -321,7 +321,7 @@ impl<V: Vfs + Clone> Db<V> {
321321
})),
322322
poisoned_commit: parking_lot::Mutex::new(None),
323323
free_page_cache: Arc::new(parking_lot::Mutex::new(Vec::new())),
324-
free_page_consumed: Arc::new(parking_lot::Mutex::new(Vec::new())),
324+
free_page_consumed: Arc::new(parking_lot::Mutex::new(rustc_hash::FxHashSet::default())),
325325
#[cfg(test)]
326326
visibility_test_hook: parking_lot::Mutex::new(None),
327327
#[cfg(test)]

src/txn/write/commit.rs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -144,9 +144,7 @@ impl<V: Vfs + Clone> WriteTxn<'_, V> {
144144
.collect();
145145
// Pages the allocator reused from the cache this txn — they were free
146146
// before, now hold live committed data, so they leave the free-list.
147-
let consumed: HashSet<u64> = std::mem::take(&mut *self.db.free_page_consumed.lock())
148-
.into_iter()
149-
.collect();
147+
let consumed = std::mem::take(&mut *self.db.free_page_consumed.lock());
150148

151149
// The scanned window after this commit: its entries minus the ones
152150
// reused, kept with their original freeing-commit tag. Every id in

0 commit comments

Comments
 (0)