Skip to content

Commit eb617be

Browse files
committed
Store non-full pages in a BTreeSet, not a Vec
And sort them by number of available var-len granules. This prevents an accidentally quadratic behavior where, for a table where the average row contains many var-len granules, after inserting a large number of rows, there would be a large number of pages in `non_full_pages` each of which had enough space for at least one fixed-len row part, but insufficient space for an actual row in practice due to insufficient var-len granules. Each insertion would then do a linear scan over `non_full_pages` before either inserting into the last page or allocating a new page which went to the end. Now, non-full pages are stored in a `BTreeSet` sorted by the number of free var-len granules, and the search for a useable page is done with a `BTreeSet::range` iterator for only the pages with enough granules. I think there may still be an off-by-one-ish bug here, where a page may have enough bytes in the gap that it could either store the fixed-len part or the var-len granules, but not both, but this fix hopefully will suffice for now.
1 parent 1dd621d commit eb617be

3 files changed

Lines changed: 37 additions & 10 deletions

File tree

crates/memory-usage/src/lib.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,13 @@ impl<K: MemoryUsage, V: MemoryUsage> MemoryUsage for std::collections::BTreeMap<
126126
}
127127
}
128128

129+
impl<T: MemoryUsage> MemoryUsage for std::collections::BTreeSet<T> {
130+
fn heap_usage(&self) -> usize {
131+
// NB: this is best-effort, since we don't have a `capacity()` method on `BTreeMap`.
132+
self.len() * mem::size_of::<T>() + self.iter().map(|t| t.heap_usage()).sum::<usize>()
133+
}
134+
}
135+
129136
#[cfg(feature = "smallvec")]
130137
impl<A: smallvec::Array> MemoryUsage for smallvec::SmallVec<A>
131138
where

crates/table/src/page.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,13 @@ impl PageHeader {
382382
pub(super) fn present_rows_storage_ptr_for_test(&self) -> *const () {
383383
self.fixed.present_rows.storage().as_ptr().cast()
384384
}
385+
386+
/// Returns the number of var-len granules available for allocation,
387+
/// including those in the "gap" between the fixed-len and var-len part of the page.
388+
fn available_var_len_granules(&self) -> usize {
389+
self.var.freelist_len as usize
390+
+ VarLenGranule::space_to_granules(gap_remaining_size(self.var.first, self.fixed.last))
391+
}
385392
}
386393

387394
/// Fixed-length row portions must be at least large enough to store a `FreeCellRef`.
@@ -1195,6 +1202,12 @@ impl Page {
11951202
self.header.var.num_granules as usize
11961203
}
11971204

1205+
/// Returns the number of var-len granules free to store data,
1206+
/// including those in the "gap" between the fixed-len and var-len part of the page.
1207+
pub fn available_var_len_granules(&self) -> usize {
1208+
self.header.available_var_len_granules()
1209+
}
1210+
11981211
#[cfg(test)]
11991212
/// # Safety
12001213
///

crates/table/src/pages.rs

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use super::var_len::VarLenMembers;
99
use core::ops::{ControlFlow, Deref, Index, IndexMut};
1010
use spacetimedb_sats::layout::Size;
1111
use spacetimedb_sats::memory_usage::MemoryUsage;
12+
use std::collections::BTreeSet;
1213
use std::ops::DerefMut;
1314
use thiserror::Error;
1415

@@ -39,8 +40,10 @@ impl IndexMut<PageIndex> for Pages {
3940
pub struct Pages {
4041
/// The collection of pages under management.
4142
pages: Vec<Box<Page>>,
42-
/// The set of pages that aren't yet full.
43-
non_full_pages: Vec<PageIndex>,
43+
44+
/// The set of pages that aren't yet full,
45+
/// sorted by the number of var-len granules available in each page.
46+
non_full_pages: BTreeSet<(usize, PageIndex)>,
4447
}
4548

4649
impl MemoryUsage for Pages {
@@ -78,7 +81,9 @@ impl Pages {
7881
page.clear();
7982
}
8083
// Mark every page non-full.
81-
self.non_full_pages = (0..self.pages.len()).map(|idx| PageIndex(idx as u64)).collect();
84+
self.non_full_pages = (0..self.pages.len())
85+
.map(|idx| (self.pages[idx].available_var_len_granules(), PageIndex(idx as u64)))
86+
.collect();
8287
}
8388

8489
/// Get a reference to fixed-len row data.
@@ -113,14 +118,15 @@ impl Pages {
113118

114119
/// Mark the page at `idx` as non-full.
115120
pub fn mark_page_non_full(&mut self, idx: PageIndex) {
116-
self.non_full_pages.push(idx);
121+
let available_granules = self.pages[idx.idx()].available_var_len_granules();
122+
self.non_full_pages.insert((available_granules, idx));
117123
}
118124

119125
/// If the page at `page_index` is not full,
120126
/// add it to the non-full set so that later insertions can access it.
121127
pub fn maybe_mark_page_non_full(&mut self, page_index: PageIndex, fixed_row_size: Size) {
122128
if !self[page_index].is_full(fixed_row_size) {
123-
self.non_full_pages.push(page_index);
129+
self.mark_page_non_full(page_index);
124130
}
125131
}
126132

@@ -151,14 +157,13 @@ impl Pages {
151157
fixed_row_size: Size,
152158
num_var_len_granules: usize,
153159
) -> Result<PageIndex, Error> {
154-
if let Some((page_idx_idx, page_idx)) = self
160+
if let Some((page_num_free_granules, page_idx)) = self
155161
.non_full_pages
156-
.iter()
162+
.range((num_var_len_granules, PageIndex(0))..)
157163
.copied()
158-
.enumerate()
159164
.find(|(_, page_idx)| self[*page_idx].has_space_for_row(fixed_row_size, num_var_len_granules))
160165
{
161-
self.non_full_pages.swap_remove(page_idx_idx);
166+
self.non_full_pages.remove(&(page_num_free_granules, page_idx));
162167
return Ok(page_idx);
163168
}
164169

@@ -358,7 +363,9 @@ impl Pages {
358363
self.non_full_pages = pages
359364
.iter()
360365
.enumerate()
361-
.filter_map(|(idx, page)| (!page.is_full(fixed_row_size)).then_some(PageIndex(idx as _)))
366+
.filter_map(|(idx, page)| {
367+
(!page.is_full(fixed_row_size)).then_some((page.available_var_len_granules(), PageIndex(idx as _)))
368+
})
362369
.collect();
363370
self.pages = pages;
364371
}

0 commit comments

Comments
 (0)