From 641df7d7e96365554d64b125759b80d289aef8c5 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Wed, 9 Jul 2025 18:50:26 +0300 Subject: [PATCH] improve my mental health by finally refactoring .cell_get() --- core/storage/btree.rs | 455 +++++---------------------------- core/storage/sqlite3_ondisk.rs | 37 +-- 2 files changed, 73 insertions(+), 419 deletions(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index efd65d62d..64db9038c 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -674,12 +674,7 @@ impl BTreeCursor { } let cell_idx = self.stack.current_cell_index() as usize; - let cell = contents.cell_get( - cell_idx, - payload_overflow_threshold_max(contents.page_type(), self.usable_space() as u16), - payload_overflow_threshold_min(contents.page_type(), self.usable_space() as u16), - self.usable_space(), - )?; + let cell = contents.cell_get(cell_idx, self.usable_space())?; match cell { BTreeCell::TableInteriorCell(TableInteriorCell { @@ -872,14 +867,7 @@ impl BTreeCursor { } let usable_size = self.usable_space(); - let cell = contents - .cell_get( - cell_idx, - payload_overflow_threshold_max(contents.page_type(), usable_size as u16), - payload_overflow_threshold_min(contents.page_type(), usable_size as u16), - usable_size, - ) - .unwrap(); + let cell = contents.cell_get(cell_idx, usable_size).unwrap(); let (payload, payload_size, first_overflow_page) = match cell { BTreeCell::TableLeafCell(cell) => { @@ -1216,12 +1204,7 @@ impl BTreeCursor { } turso_assert!(cell_idx < contents.cell_count(), "cell index out of bounds"); - let cell = contents.cell_get( - cell_idx, - payload_overflow_threshold_max(contents.page_type(), self.usable_space() as u16), - payload_overflow_threshold_min(contents.page_type(), self.usable_space() as u16), - self.usable_space(), - )?; + let cell = contents.cell_get(cell_idx, self.usable_space())?; match &cell { BTreeCell::TableInteriorCell(TableInteriorCell { _left_child_page, @@ -1515,18 +1498,8 @@ impl BTreeCursor { } } }; - let matching_cell = contents.cell_get( - leftmost_matching_cell, - payload_overflow_threshold_max( - contents.page_type(), - self.usable_space() as u16, - ), - payload_overflow_threshold_min( - contents.page_type(), - self.usable_space() as u16, - ), - self.usable_space(), - )?; + let matching_cell = + contents.cell_get(leftmost_matching_cell, self.usable_space())?; self.stack.set_cell_index(leftmost_matching_cell as i32); // we don't advance in case of forward iteration and index tree internal nodes because we will visit this node going up. // in backwards iteration, we must retreat because otherwise we would unnecessarily visit this node again. @@ -1554,18 +1527,7 @@ impl BTreeCursor { let cur_cell_idx = (min + max) >> 1; // rustc generates extra insns for (min+max)/2 due to them being isize. we know min&max are >=0 here. self.stack.set_cell_index(cur_cell_idx as i32); - let cell = contents.cell_get( - cur_cell_idx as usize, - payload_overflow_threshold_max( - contents.page_type(), - self.usable_space() as u16, - ), - payload_overflow_threshold_min( - contents.page_type(), - self.usable_space() as u16, - ), - self.usable_space(), - )?; + let cell = contents.cell_get(cur_cell_idx as usize, self.usable_space())?; let BTreeCell::IndexInteriorCell(IndexInteriorCell { payload, payload_size, @@ -1847,12 +1809,7 @@ impl BTreeCursor { let page = page.get(); let contents = page.get().contents.as_ref().unwrap(); let cur_cell_idx = self.stack.current_cell_index() as usize; - let cell = contents.cell_get( - cur_cell_idx, - payload_overflow_threshold_max(contents.page_type(), self.usable_space() as u16), - payload_overflow_threshold_min(contents.page_type(), self.usable_space() as u16), - self.usable_space(), - )?; + let cell = contents.cell_get(cur_cell_idx, self.usable_space())?; let BTreeCell::IndexInteriorCell(IndexInteriorCell { payload, first_overflow_page, @@ -1946,12 +1903,7 @@ impl BTreeCursor { let cur_cell_idx = (min + max) >> 1; // rustc generates extra insns for (min+max)/2 due to them being isize. we know min&max are >=0 here. self.stack.set_cell_index(cur_cell_idx as i32); - let cell = contents.cell_get( - cur_cell_idx as usize, - payload_overflow_threshold_max(contents.page_type(), self.usable_space() as u16), - payload_overflow_threshold_min(contents.page_type(), self.usable_space() as u16), - self.usable_space(), - )?; + let cell = contents.cell_get(cur_cell_idx as usize, self.usable_space())?; let BTreeCell::IndexLeafCell(IndexLeafCell { payload, first_overflow_page, @@ -2141,12 +2093,10 @@ impl BTreeCursor { // if the cell index is less than the total cells, check: if its an existing // rowid, we are going to update / overwrite the cell if cell_idx < page.get().get_contents().cell_count() { - let cell = page.get().get_contents().cell_get( - cell_idx, - payload_overflow_threshold_max(page_type, self.usable_space() as u16), - payload_overflow_threshold_min(page_type, self.usable_space() as u16), - self.usable_space(), - )?; + let cell = page + .get() + .get_contents() + .cell_get(cell_idx, self.usable_space())?; match cell { BTreeCell::TableLeafCell(tbl_leaf) => { if tbl_leaf._rowid == bkey.to_rowid() { @@ -2431,14 +2381,6 @@ impl BTreeCursor { } else { let (start_of_cell, _) = parent_contents.cell_get_raw_region( first_cell_divider + sibling_pointer, - payload_overflow_threshold_max( - parent_contents.page_type(), - self.usable_space() as u16, - ), - payload_overflow_threshold_min( - parent_contents.page_type(), - self.usable_space() as u16, - ), self.usable_space(), ); let buf = parent_contents.as_ptr().as_mut_ptr(); @@ -2474,18 +2416,7 @@ impl BTreeCursor { break; } let next_cell_divider = i + first_cell_divider - 1; - pgno = match parent_contents.cell_get( - next_cell_divider, - payload_overflow_threshold_max( - parent_contents.page_type(), - self.usable_space() as u16, - ), - payload_overflow_threshold_min( - parent_contents.page_type(), - self.usable_space() as u16, - ), - self.usable_space(), - )? { + pgno = match parent_contents.cell_get(next_cell_divider, self.usable_space())? { BTreeCell::TableInteriorCell(table_interior_cell) => { table_interior_cell._left_child_page } @@ -2573,18 +2504,8 @@ impl BTreeCursor { } // Since we know we have a left sibling, take the divider that points to left sibling of this page let cell_idx = balance_info.first_divider_cell + i; - let (cell_start, cell_len) = parent_contents.cell_get_raw_region( - cell_idx, - payload_overflow_threshold_max( - parent_contents.page_type(), - self.usable_space() as u16, - ), - payload_overflow_threshold_min( - parent_contents.page_type(), - self.usable_space() as u16, - ), - self.usable_space(), - ); + let (cell_start, cell_len) = + parent_contents.cell_get_raw_region(cell_idx, self.usable_space()); let buf = parent_contents.as_ptr(); let cell_buf = &buf[cell_start..cell_start + cell_len]; max_cells += 1; @@ -2636,18 +2557,8 @@ impl BTreeCursor { let old_page_contents = old_page.get_contents(); debug_validate_cells!(&old_page_contents, self.usable_space() as u16); for cell_idx in 0..old_page_contents.cell_count() { - let (cell_start, cell_len) = old_page_contents.cell_get_raw_region( - cell_idx, - payload_overflow_threshold_max( - old_page_contents.page_type(), - self.usable_space() as u16, - ), - payload_overflow_threshold_min( - old_page_contents.page_type(), - self.usable_space() as u16, - ), - self.usable_space(), - ); + let (cell_start, cell_len) = + old_page_contents.cell_get_raw_region(cell_idx, self.usable_space()); let buf = old_page_contents.as_ptr(); let cell_buf = &mut buf[cell_start..cell_start + cell_len]; // TODO(pere): make this reference and not copy @@ -3269,18 +3180,8 @@ impl BTreeCursor { page: &std::sync::Arc, ) { let left_pointer = if parent_contents.overflow_cells.is_empty() { - let (cell_start, cell_len) = parent_contents.cell_get_raw_region( - balance_info.first_divider_cell + i, - payload_overflow_threshold_max( - parent_contents.page_type(), - self.usable_space() as u16, - ), - payload_overflow_threshold_min( - parent_contents.page_type(), - self.usable_space() as u16, - ), - self.usable_space(), - ); + let (cell_start, cell_len) = parent_contents + .cell_get_raw_region(balance_info.first_divider_cell + i, self.usable_space()); tracing::debug!( "balance_non_root(cell_start={}, cell_len={})", cell_start, @@ -3325,18 +3226,7 @@ impl BTreeCursor { let mut current_index_cell = 0; for cell_idx in 0..parent_contents.cell_count() { let cell = parent_contents - .cell_get( - cell_idx, - payload_overflow_threshold_max( - parent_contents.page_type(), - self.usable_space() as u16, - ), - payload_overflow_threshold_min( - parent_contents.page_type(), - self.usable_space() as u16, - ), - self.usable_space(), - ) + .cell_get(cell_idx, self.usable_space()) .unwrap(); match cell { BTreeCell::TableInteriorCell(table_interior_cell) => { @@ -3374,18 +3264,8 @@ impl BTreeCursor { debug_validate_cells!(contents, self.usable_space() as u16); // Cells are distributed in order for cell_idx in 0..contents.cell_count() { - let (cell_start, cell_len) = contents.cell_get_raw_region( - cell_idx, - payload_overflow_threshold_max( - contents.page_type(), - self.usable_space() as u16, - ), - payload_overflow_threshold_min( - contents.page_type(), - self.usable_space() as u16, - ), - self.usable_space(), - ); + let (cell_start, cell_len) = + contents.cell_get_raw_region(cell_idx, self.usable_space()); let buf = contents.as_ptr(); let cell_buf = to_static_buf(&mut buf[cell_start..cell_start + cell_len]); let cell_buf_in_array = &cells_debug[current_index_cell]; @@ -3399,16 +3279,8 @@ impl BTreeCursor { let cell = crate::storage::sqlite3_ondisk::read_btree_cell( cell_buf, - &page_type, + contents, 0, - payload_overflow_threshold_max( - parent_contents.page_type(), - self.usable_space() as u16, - ), - payload_overflow_threshold_min( - parent_contents.page_type(), - self.usable_space() as u16, - ), self.usable_space(), ) .unwrap(); @@ -3542,31 +3414,11 @@ impl BTreeCursor { for (parent_cell_idx, cell_buf_in_array) in cells_debug.iter().enumerate().take(contents.cell_count()) { - let (parent_cell_start, parent_cell_len) = parent_contents.cell_get_raw_region( - parent_cell_idx, - payload_overflow_threshold_max( - parent_contents.page_type(), - self.usable_space() as u16, - ), - payload_overflow_threshold_min( - parent_contents.page_type(), - self.usable_space() as u16, - ), - self.usable_space(), - ); + let (parent_cell_start, parent_cell_len) = + parent_contents.cell_get_raw_region(parent_cell_idx, self.usable_space()); - let (cell_start, cell_len) = contents.cell_get_raw_region( - parent_cell_idx, - payload_overflow_threshold_max( - contents.page_type(), - self.usable_space() as u16, - ), - payload_overflow_threshold_min( - contents.page_type(), - self.usable_space() as u16, - ), - self.usable_space(), - ); + let (cell_start, cell_len) = + contents.cell_get_raw_region(parent_cell_idx, self.usable_space()); let buf = contents.as_ptr(); let cell_buf = to_static_buf(&mut buf[cell_start..cell_start + cell_len]); @@ -3624,18 +3476,8 @@ impl BTreeCursor { } // check if overflow // check if right pointer, this is the last page. Do we update rightmost pointer and defragment moves it? - let (cell_start, cell_len) = parent_contents.cell_get_raw_region( - cell_divider_idx, - payload_overflow_threshold_max( - parent_contents.page_type(), - self.usable_space() as u16, - ), - payload_overflow_threshold_min( - parent_contents.page_type(), - self.usable_space() as u16, - ), - self.usable_space(), - ); + let (cell_start, cell_len) = + parent_contents.cell_get_raw_region(cell_divider_idx, self.usable_space()); let cell_left_pointer = read_u32(&parent_buf[cell_start..cell_start + cell_len], 0); if cell_left_pointer != page.get().id as u32 { tracing::error!("balance_non_root(cell_divider_left_pointer, should point to page_id={}, but points to {}, divider_cell={}, overflow_cells_parent={})", @@ -3657,32 +3499,13 @@ impl BTreeCursor { to_static_buf(&mut cells_debug[current_index_cell - 1]); let cell = crate::storage::sqlite3_ondisk::read_btree_cell( cell_buf, - &page_type, + contents, 0, - payload_overflow_threshold_max( - parent_contents.page_type(), - self.usable_space() as u16, - ), - payload_overflow_threshold_min( - parent_contents.page_type(), - self.usable_space() as u16, - ), self.usable_space(), ) .unwrap(); let parent_cell = parent_contents - .cell_get( - cell_divider_idx, - payload_overflow_threshold_max( - parent_contents.page_type(), - self.usable_space() as u16, - ), - payload_overflow_threshold_min( - parent_contents.page_type(), - self.usable_space() as u16, - ), - self.usable_space(), - ) + .cell_get(cell_divider_idx, self.usable_space()) .unwrap(); let rowid = match cell { BTreeCell::TableLeafCell(table_leaf_cell) => table_leaf_cell._rowid, @@ -3729,18 +3552,8 @@ impl BTreeCursor { } continue; } - let (parent_cell_start, parent_cell_len) = parent_contents.cell_get_raw_region( - cell_divider_idx, - payload_overflow_threshold_max( - parent_contents.page_type(), - self.usable_space() as u16, - ), - payload_overflow_threshold_min( - parent_contents.page_type(), - self.usable_space() as u16, - ), - self.usable_space(), - ); + let (parent_cell_start, parent_cell_len) = + parent_contents.cell_get_raw_region(cell_divider_idx, self.usable_space()); let cell_buf_in_array = &cells_debug[current_index_cell]; let left_pointer = read_u32( &parent_buf[parent_cell_start..parent_cell_start + parent_cell_len], @@ -3888,12 +3701,7 @@ impl BTreeCursor { while low <= high && cell_count > 0 { let mid = low + (high - low) / 2; self.find_cell_state.set((low, high)); - let cell = match page.cell_get( - mid, - payload_overflow_threshold_max(page.page_type(), self.usable_space() as u16), - payload_overflow_threshold_min(page.page_type(), self.usable_space() as u16), - self.usable_space(), - ) { + let cell = match page.cell_get(mid, self.usable_space()) { Ok(c) => c, Err(e) => return Err(e), }; @@ -4076,12 +3884,7 @@ impl BTreeCursor { let page = page.get(); let contents = page.get_contents(); let cell_idx = self.stack.current_cell_index(); - let cell = contents.cell_get( - cell_idx as usize, - payload_overflow_threshold_max(contents.page_type(), self.usable_space() as u16), - payload_overflow_threshold_min(contents.page_type(), self.usable_space() as u16), - self.usable_space(), - )?; + let cell = contents.cell_get(cell_idx as usize, self.usable_space())?; if page_type.is_table() { let BTreeCell::TableLeafCell(TableLeafCell { _rowid, _payload, .. @@ -4149,12 +3952,7 @@ impl BTreeCursor { let page = page.get(); let contents = page.get_contents(); let cell_idx = self.stack.current_cell_index(); - let cell = contents.cell_get( - cell_idx as usize, - payload_overflow_threshold_max(contents.page_type(), self.usable_space() as u16), - payload_overflow_threshold_min(contents.page_type(), self.usable_space() as u16), - self.usable_space(), - )?; + let cell = contents.cell_get(cell_idx as usize, self.usable_space())?; let (payload, payload_size, first_overflow_page) = match cell { BTreeCell::TableLeafCell(TableLeafCell { _rowid, @@ -4359,18 +4157,7 @@ impl BTreeCursor { cell_idx ); - let cell = contents.cell_get( - cell_idx, - payload_overflow_threshold_max( - contents.page_type(), - self.usable_space() as u16, - ), - payload_overflow_threshold_min( - contents.page_type(), - self.usable_space() as u16, - ), - self.usable_space(), - )?; + let cell = contents.cell_get(cell_idx, self.usable_space())?; let original_child_pointer = match &cell { BTreeCell::TableInteriorCell(interior) => Some(interior._left_child_page), @@ -4437,18 +4224,8 @@ impl BTreeCursor { assert!(leaf_contents.is_leaf()); assert!(leaf_contents.cell_count() > 0); let leaf_cell_idx = leaf_contents.cell_count() - 1; - let last_cell_on_child_page = leaf_contents.cell_get( - leaf_cell_idx, - payload_overflow_threshold_max( - leaf_contents.page_type(), - self.usable_space() as u16, - ), - payload_overflow_threshold_min( - leaf_contents.page_type(), - self.usable_space() as u16, - ), - self.usable_space(), - )?; + let last_cell_on_child_page = + leaf_contents.cell_get(leaf_cell_idx, self.usable_space())?; let mut cell_payload: Vec = Vec::new(); let child_pointer = @@ -4803,18 +4580,7 @@ impl BTreeCursor { // We have not yet processed all cells in this page // Get the current cell - let cell = contents.cell_get( - cell_idx as usize, - payload_overflow_threshold_max( - contents.page_type(), - self.usable_space() as u16, - ), - payload_overflow_threshold_min( - contents.page_type(), - self.usable_space() as u16, - ), - self.usable_space(), - )?; + let cell = contents.cell_get(cell_idx as usize, self.usable_space())?; match contents.is_leaf() { // For a leaf cell, clear the overflow pages associated with this cell @@ -4931,12 +4697,7 @@ impl BTreeCursor { let (old_offset, old_local_size) = { let page_ref = page_ref.get(); let page = page_ref.get().contents.as_ref().unwrap(); - page.cell_get_raw_region( - cell_idx, - payload_overflow_threshold_max(page_type, self.usable_space() as u16), - payload_overflow_threshold_min(page_type, self.usable_space() as u16), - self.usable_space(), - ) + page.cell_get_raw_region(cell_idx, self.usable_space()) }; // if it all fits in local space and old_local_size is enough, do an in-place overwrite @@ -5064,18 +4825,7 @@ impl BTreeCursor { self.stack.push(mem_page); } else { // Move to child left page - let cell = contents.cell_get( - cell_idx, - payload_overflow_threshold_max( - contents.page_type(), - self.usable_space() as u16, - ), - payload_overflow_threshold_min( - contents.page_type(), - self.usable_space() as u16, - ), - self.usable_space(), - )?; + let cell = contents.cell_get(cell_idx, self.usable_space())?; match cell { BTreeCell::TableInteriorCell(TableInteriorCell { @@ -5273,12 +5023,8 @@ pub fn integrity_check( // have seen. let mut next_rowid = max_intkey; for cell_idx in (0..contents.cell_count()).rev() { - let (cell_start, cell_length) = contents.cell_get_raw_region( - cell_idx, - payload_overflow_threshold_max(contents.page_type(), usable_space), - payload_overflow_threshold_min(contents.page_type(), usable_space), - usable_space as usize, - ); + let (cell_start, cell_length) = + contents.cell_get_raw_region(cell_idx, usable_space as usize); if cell_start < contents.cell_content_area() as usize || cell_start > usable_space as usize - 4 { @@ -5302,12 +5048,7 @@ pub fn integrity_check( }); } coverage_checker.add_cell(cell_start, cell_start + cell_length); - let cell = contents.cell_get( - cell_idx, - payload_overflow_threshold_max(contents.page_type(), usable_space), - payload_overflow_threshold_min(contents.page_type(), usable_space), - usable_space as usize, - )?; + let cell = contents.cell_get(cell_idx, usable_space as usize)?; match cell { BTreeCell::TableInteriorCell(table_interior_cell) => { state.page_stack.push(IntegrityCheckPageEntry { @@ -6110,12 +5851,7 @@ fn defragment_page(page: &PageContent, usable_space: u16) { assert!(pc <= last_cell); - let (_, size) = cloned_page.cell_get_raw_region( - i, - payload_overflow_threshold_max(page.page_type(), usable_space), - payload_overflow_threshold_min(page.page_type(), usable_space), - usable_space as usize, - ); + let (_, size) = cloned_page.cell_get_raw_region(i, usable_space as usize); let size = size as u16; cbrk -= size; if cbrk < first_cell || pc + size > usable_space { @@ -6148,12 +5884,7 @@ fn defragment_page(page: &PageContent, usable_space: u16) { /// Only enabled in debug mode, where we ensure that all cells are valid. fn debug_validate_cells_core(page: &PageContent, usable_space: u16) { for i in 0..page.cell_count() { - let (offset, size) = page.cell_get_raw_region( - i, - payload_overflow_threshold_max(page.page_type(), usable_space), - payload_overflow_threshold_min(page.page_type(), usable_space), - usable_space as usize, - ); + let (offset, size) = page.cell_get_raw_region(i, usable_space as usize); let buf = &page.as_ptr()[offset..offset + size]; // E.g. the following table btree cell may just have two bytes: // Payload size 0 (stored as SerialTypeKind::ConstInt0) @@ -6437,7 +6168,7 @@ fn fill_cell_payload( /// - Give a minimum fanout of 4 for index b-trees /// - Ensure enough payload is on the b-tree page that the record header can usually be accessed /// without consulting an overflow page -fn payload_overflow_threshold_max(page_type: PageType, usable_space: u16) -> usize { +pub fn payload_overflow_threshold_max(page_type: PageType, usable_space: u16) -> usize { match page_type { PageType::IndexInterior | PageType::IndexLeaf => { ((usable_space as usize - 12) * 64 / 255) - 23 // Index page formula @@ -6457,7 +6188,7 @@ fn payload_overflow_threshold_max(page_type: PageType, usable_space: u16) -> usi /// - Otherwise: store M bytes on page /// /// The remaining bytes are stored on overflow pages in both cases. -fn payload_overflow_threshold_min(_page_type: PageType, usable_space: u16) -> usize { +pub fn payload_overflow_threshold_min(_page_type: PageType, usable_space: u16) -> usize { // Same formula for all page types ((usable_space as usize - 12) * 32 / 255) - 23 } @@ -6465,12 +6196,7 @@ fn payload_overflow_threshold_min(_page_type: PageType, usable_space: u16) -> us /// Drop a cell from a page. /// This is done by freeing the range of bytes that the cell occupies. fn drop_cell(page: &mut PageContent, cell_idx: usize, usable_space: u16) -> Result<()> { - let (cell_start, cell_len) = page.cell_get_raw_region( - cell_idx, - payload_overflow_threshold_max(page.page_type(), usable_space), - payload_overflow_threshold_min(page.page_type(), usable_space), - usable_space as usize, - ); + let (cell_start, cell_len) = page.cell_get_raw_region(cell_idx, usable_space as usize); free_cell_range(page, cell_start as u16, cell_len as u16, usable_space)?; if page.cell_count() > 1 { shift_pointers_left(page, cell_idx); @@ -6529,10 +6255,7 @@ mod tests { use crate::{ io::BufferData, storage::{ - btree::{ - compute_free_space, fill_cell_payload, payload_overflow_threshold_max, - payload_overflow_threshold_min, - }, + btree::{compute_free_space, fill_cell_payload, payload_overflow_threshold_max}, sqlite3_ondisk::{BTreeCell, PageContent, PageType}, }, types::Value, @@ -6579,12 +6302,7 @@ mod tests { } fn ensure_cell(page: &mut PageContent, cell_idx: usize, payload: &Vec) { - let cell = page.cell_get_raw_region( - cell_idx, - payload_overflow_threshold_max(page.page_type(), 4096), - payload_overflow_threshold_min(page.page_type(), 4096), - 4096, - ); + let cell = page.cell_get_raw_region(cell_idx, 4096); tracing::trace!("cell idx={} start={} len={}", cell_idx, cell.0, cell.1); let buf = &page.as_ptr()[cell.0..cell.0 + cell.1]; assert_eq!(buf.len(), payload.len()); @@ -6680,21 +6398,13 @@ mod tests { // Pin page in order to not drop it in between page.set_dirty(); let contents = page.get().contents.as_ref().unwrap(); - let page_type = contents.page_type(); let mut previous_key = None; let mut valid = true; let mut depth = None; debug_validate_cells!(contents, pager.usable_space() as u16); let mut child_pages = Vec::new(); for cell_idx in 0..contents.cell_count() { - let cell = contents - .cell_get( - cell_idx, - payload_overflow_threshold_max(page_type, 4096), - payload_overflow_threshold_min(page_type, 4096), - cursor.usable_space(), - ) - .unwrap(); + let cell = contents.cell_get(cell_idx, cursor.usable_space()).unwrap(); let current_depth = match cell { BTreeCell::TableLeafCell(..) => 1, BTreeCell::TableInteriorCell(TableInteriorCell { @@ -6797,18 +6507,10 @@ mod tests { // Pin page in order to not drop it in between loading of different pages. If not contents will be a dangling reference. page.set_dirty(); let contents = page.get().contents.as_ref().unwrap(); - let page_type = contents.page_type(); let mut current = Vec::new(); let mut child = Vec::new(); for cell_idx in 0..contents.cell_count() { - let cell = contents - .cell_get( - cell_idx, - payload_overflow_threshold_max(page_type, 4096), - payload_overflow_threshold_min(page_type, 4096), - cursor.usable_space(), - ) - .unwrap(); + let cell = contents.cell_get(cell_idx, cursor.usable_space()).unwrap(); match cell { BTreeCell::TableInteriorCell(cell) => { current.push(format!( @@ -7749,12 +7451,7 @@ mod tests { continue; } let cell_idx = rng.next_u64() as usize % page.cell_count(); - let (_, len) = page.cell_get_raw_region( - cell_idx, - payload_overflow_threshold_max(page.page_type(), 4096), - payload_overflow_threshold_min(page.page_type(), 4096), - usable_space as usize, - ); + let (_, len) = page.cell_get_raw_region(cell_idx, usable_space as usize); drop_cell(page, cell_idx, usable_space).unwrap(); total_size -= len as u16 + 2; cells.remove(cell_idx); @@ -7830,12 +7527,7 @@ mod tests { continue; } let cell_idx = rng.next_u64() as usize % page.cell_count(); - let (_, len) = page.cell_get_raw_region( - cell_idx, - payload_overflow_threshold_max(page.page_type(), 4096), - payload_overflow_threshold_min(page.page_type(), 4096), - usable_space as usize, - ); + let (_, len) = page.cell_get_raw_region(cell_idx, usable_space as usize); drop_cell(page, cell_idx, usable_space).unwrap(); total_size -= len as u16 + 2; cells.remove(cell_idx); @@ -7985,12 +7677,7 @@ mod tests { assert_eq!(page.cell_count(), 1); defragment_page(page, usable_space); assert_eq!(page.cell_count(), 1); - let (start, len) = page.cell_get_raw_region( - 0, - payload_overflow_threshold_max(page.page_type(), 4096), - payload_overflow_threshold_min(page.page_type(), 4096), - usable_space as usize, - ); + let (start, len) = page.cell_get_raw_region(0, usable_space as usize); let buf = page.as_ptr(); assert_eq!(&payload, &buf[start..start + len]); } @@ -8021,12 +7708,7 @@ mod tests { let payload = add_record(0, 0, page, record, &conn); assert_eq!(page.cell_count(), 1); - let (start, len) = page.cell_get_raw_region( - 0, - payload_overflow_threshold_max(page.page_type(), 4096), - payload_overflow_threshold_min(page.page_type(), 4096), - usable_space as usize, - ); + let (start, len) = page.cell_get_raw_region(0, usable_space as usize); let buf = page.as_ptr(); assert_eq!(&payload, &buf[start..start + len]); } @@ -8058,12 +7740,7 @@ mod tests { let payload = add_record(0, 0, page, record, &conn); assert_eq!(page.cell_count(), 1); - let (start, len) = page.cell_get_raw_region( - 0, - payload_overflow_threshold_max(page.page_type(), 4096), - payload_overflow_threshold_min(page.page_type(), 4096), - usable_space as usize, - ); + let (start, len) = page.cell_get_raw_region(0, usable_space as usize); let buf = page.as_ptr(); assert_eq!(&payload, &buf[start..start + len]); } @@ -8624,12 +8301,7 @@ mod tests { let contents = page.get_contents(); for cell_idx in 0..contents.cell_count() { let buf = contents.as_ptr(); - let (start, len) = contents.cell_get_raw_region( - cell_idx, - payload_overflow_threshold_max(contents.page_type(), 4096), - payload_overflow_threshold_min(contents.page_type(), 4096), - pager.usable_space(), - ); + let (start, len) = contents.cell_get_raw_region(cell_idx, pager.usable_space()); cell_array .cells .push(to_static_buf(&mut buf[start..start + len])); @@ -8668,12 +8340,7 @@ mod tests { let mut cell_idx_cloned = if prefix { size } else { 0 }; for cell_idx in 0..contents.cell_count() { let buf = contents.as_ptr(); - let (start, len) = contents.cell_get_raw_region( - cell_idx, - payload_overflow_threshold_max(contents.page_type(), 4096), - payload_overflow_threshold_min(contents.page_type(), 4096), - pager.usable_space(), - ); + let (start, len) = contents.cell_get_raw_region(cell_idx, pager.usable_space()); let cell_in_page = &buf[start..start + len]; let cell_in_array = &cells_cloned[cell_idx_cloned]; assert_eq!(cell_in_page, cell_in_array); diff --git a/core/storage/sqlite3_ondisk.rs b/core/storage/sqlite3_ondisk.rs index 6fd6d219b..f1c7cbeb7 100644 --- a/core/storage/sqlite3_ondisk.rs +++ b/core/storage/sqlite3_ondisk.rs @@ -56,6 +56,7 @@ use crate::storage::btree::offset::{ BTREE_CELL_CONTENT_AREA, BTREE_CELL_COUNT, BTREE_FIRST_FREEBLOCK, BTREE_FRAGMENTED_BYTES_COUNT, BTREE_PAGE_TYPE, BTREE_RIGHTMOST_PTR, }; +use crate::storage::btree::{payload_overflow_threshold_max, payload_overflow_threshold_min}; use crate::storage::buffer_pool::BufferPool; use crate::storage::database::DatabaseStorage; use crate::storage::pager::Pager; @@ -536,13 +537,7 @@ impl PageContent { } } - pub fn cell_get( - &self, - idx: usize, - payload_overflow_threshold_max: usize, - payload_overflow_threshold_min: usize, - usable_size: usize, - ) -> Result { + pub fn cell_get(&self, idx: usize, usable_size: usize) -> Result { tracing::trace!("cell_get(idx={})", idx); let buf = self.as_ptr(); @@ -560,14 +555,7 @@ impl PageContent { // SAFETY: this buffer is valid as long as the page is alive. We could store the page in the cell and do some lifetime magic // but that is extra memory for no reason at all. Just be careful like in the old times :). let static_buf: &'static [u8] = unsafe { std::mem::transmute::<&[u8], &'static [u8]>(buf) }; - read_btree_cell( - static_buf, - &self.page_type(), - cell_pointer, - payload_overflow_threshold_max, - payload_overflow_threshold_min, - usable_size, - ) + read_btree_cell(static_buf, self, cell_pointer, usable_size) } /// Read the rowid of a table interior cell. @@ -629,13 +617,7 @@ impl PageContent { } /// Get region(start end length) of a cell's payload - pub fn cell_get_raw_region( - &self, - idx: usize, - payload_overflow_threshold_max: usize, - payload_overflow_threshold_min: usize, - usable_size: usize, - ) -> (usize, usize) { + pub fn cell_get_raw_region(&self, idx: usize, usable_size: usize) -> (usize, usize) { let buf = self.as_ptr(); let ncells = self.cell_count(); let (cell_pointer_array_start, _) = self.cell_pointer_array_offset_and_size(); @@ -643,6 +625,10 @@ impl PageContent { let cell_pointer = cell_pointer_array_start + (idx * CELL_POINTER_SIZE_BYTES); let cell_pointer = self.read_u16_no_offset(cell_pointer) as usize; let start = cell_pointer; + let payload_overflow_threshold_max = + payload_overflow_threshold_max(self.page_type(), usable_size as u16); + let payload_overflow_threshold_min = + payload_overflow_threshold_min(self.page_type(), usable_size as u16); let len = match self.page_type() { PageType::IndexInterior => { let (len_payload, n_payload) = read_varint(&buf[cell_pointer + 4..]).unwrap(); @@ -890,12 +876,13 @@ pub struct IndexLeafCell { /// buffer input "page" is static because we want the cell to point to the data in the page in case it has any payload. pub fn read_btree_cell( page: &'static [u8], - page_type: &PageType, + page_content: &PageContent, pos: usize, - max_local: usize, - min_local: usize, usable_size: usize, ) -> Result { + let page_type = page_content.page_type(); + let max_local = payload_overflow_threshold_max(page_type, usable_size as u16); + let min_local = payload_overflow_threshold_min(page_type, usable_size as u16); match page_type { PageType::IndexInterior => { let mut pos = pos;