From c374cf0c93b2faa0f545be437302ccf0b7dec725 Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Tue, 2 Sep 2025 03:07:01 +0400 Subject: [PATCH 1/8] remove Cell/RefCell from PageStack --- core/storage/btree.rs | 132 ++++++++++++++++++------------------------ 1 file changed, 57 insertions(+), 75 deletions(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index 15d50e70c..4a88a40ef 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -595,9 +595,9 @@ impl BTreeCursor { }, overflow_state: OverflowState::Start, stack: PageStack { - current_page: Cell::new(-1), - node_states: RefCell::new([BTreeNodeState::default(); BTCURSOR_MAX_DEPTH + 1]), - stack: RefCell::new([const { None }; BTCURSOR_MAX_DEPTH + 1]), + current_page: -1, + node_states: [BTreeNodeState::default(); BTCURSOR_MAX_DEPTH + 1], + stack: [const { None }; BTCURSOR_MAX_DEPTH + 1], }, reusable_immutable_record: RefCell::new(None), index_info: None, @@ -1182,7 +1182,7 @@ impl BTreeCursor { /// Check if any ancestor pages still have cells to iterate. /// If not, traversing back up to parent is of no use because we are at the end of the tree. fn ancestor_pages_have_more_children(&self) -> bool { - let node_states = self.stack.node_states.borrow(); + let node_states = self.stack.node_states; (0..self.stack.current()) .rev() .any(|idx| !node_states[idx].is_at_end()) @@ -4586,7 +4586,6 @@ impl BTreeCursor { // Ensure we keep the parent page at the same position as before the replacement. self.stack .node_states - .borrow_mut() .get_mut(btree_depth) .expect("parent page should be on the stack") .cell_idx = cell_idx as i32; @@ -5884,9 +5883,9 @@ impl CoverageChecker { /// the parent. Using current_page + 1 or higher is undefined behaviour. struct PageStack { /// Pointer to the current page being consumed - current_page: Cell, + current_page: i32, /// List of pages in the stack. Root page will be in index 0 - pub stack: RefCell<[Option; BTCURSOR_MAX_DEPTH + 1]>, + pub stack: [Option; BTCURSOR_MAX_DEPTH + 1], /// List of cell indices in the stack. /// node_states[current_page] is the current cell index being consumed. Similarly /// node_states[current_page-1] is the cell index of the parent of the current page @@ -5894,32 +5893,21 @@ struct PageStack { /// There are two points that need special attention: /// If node_states[current_page] = -1, it indicates that the current iteration has reached the start of the current_page /// If node_states[current_page] = `cell_count`, it means that the current iteration has reached the end of the current_page - node_states: RefCell<[BTreeNodeState; BTCURSOR_MAX_DEPTH + 1]>, + node_states: [BTreeNodeState; BTCURSOR_MAX_DEPTH + 1], } impl PageStack { - fn increment_current(&self) { - self.current_page.set(self.current_page.get() + 1); - } - fn decrement_current(&self) { - assert!(self.current_page.get() > 0); - self.current_page.set(self.current_page.get() - 1); - } /// Push a new page onto the stack. /// This effectively means traversing to a child page. #[instrument(skip_all, level = Level::DEBUG, name = "pagestack::push")] - fn _push(&self, page: PageRef, starting_cell_idx: i32) { - tracing::trace!( - current = self.current_page.get(), - new_page_id = page.get().id, - ); + fn _push(&mut self, page: PageRef, starting_cell_idx: i32) { + tracing::trace!(current = self.current_page, new_page_id = page.get().id,); 'validate: { - let current = self.current_page.get(); + let current = self.current_page; if current == -1 { break 'validate; } - let stack = self.stack.borrow(); - let current_top = stack[current as usize].as_ref(); + let current_top = self.stack[current as usize].as_ref(); if let Some(current_top) = current_top { turso_assert!( current_top.get().id != page.get().id, @@ -5929,19 +5917,19 @@ impl PageStack { } } self.populate_parent_cell_count(); - self.increment_current(); - let current = self.current_page.get(); + self.current_page += 1; + assert!(self.current_page >= 0); + let current = self.current_page as usize; assert!( - current < BTCURSOR_MAX_DEPTH as i32, + current < BTCURSOR_MAX_DEPTH, "corrupted database, stack is bigger than expected" ); - assert!(current >= 0); // Pin the page to prevent it from being evicted while on the stack page.pin(); - self.stack.borrow_mut()[current as usize] = Some(page); - self.node_states.borrow_mut()[current as usize] = BTreeNodeState { + self.stack[current] = Some(page.clone()); + self.node_states[current] = BTreeNodeState { cell_idx: starting_cell_idx, cell_count: None, // we don't know the cell count yet, so we set it to None. any code pushing a child page onto the stack MUST set the parent page's cell_count. }; @@ -5953,14 +5941,13 @@ impl PageStack { /// /// This rests on the assumption that the parent page is already in memory whenever a child is pushed onto the stack. /// We currently ensure this by pinning all the pages on [PageStack] to the page cache so that they cannot be evicted. - fn populate_parent_cell_count(&self) { - let stack_empty = self.current_page.get() == -1; + fn populate_parent_cell_count(&mut self) { + let stack_empty = self.current_page == -1; if stack_empty { return; } let current = self.current(); - let stack = self.stack.borrow(); - let page = stack[current].as_ref().unwrap(); + let page = self.stack[current].as_ref().unwrap(); turso_assert!( page.is_pinned(), "parent page {} is not pinned", @@ -5973,59 +5960,59 @@ impl PageStack { ); let contents = page.get_contents(); let cell_count = contents.cell_count() as i32; - self.node_states.borrow_mut()[current].cell_count = Some(cell_count); + self.node_states[current].cell_count = Some(cell_count); } - fn push(&self, page: PageRef) { + fn push(&mut self, page: PageRef) { self._push(page, -1); } - fn push_backwards(&self, page: PageRef) { + fn push_backwards(&mut self, page: PageRef) { self._push(page, i32::MAX); } /// Pop a page off the stack. /// This effectively means traversing back up to a parent page. #[instrument(skip_all, level = Level::DEBUG, name = "pagestack::pop")] - fn pop(&self) { - let current = self.current_page.get(); + fn pop(&mut self) { + let current = self.current_page; assert!(current >= 0); tracing::trace!(current); + let current = current as usize; // Unpin the page before removing it from the stack - if let Some(page) = &self.stack.borrow()[current as usize] { + if let Some(page) = &self.stack[current] { page.unpin(); } - self.node_states.borrow_mut()[current as usize] = BTreeNodeState::default(); - self.stack.borrow_mut()[current as usize] = None; - self.decrement_current(); + self.node_states[current] = BTreeNodeState::default(); + self.stack[current] = None; + assert!(current > 0); + self.current_page -= 1; } /// Get the top page on the stack. /// This is the page that is currently being traversed. - #[instrument(skip(self), level = Level::DEBUG, name = "pagestack::top", )] + #[instrument(skip(self), level = Level::DEBUG, name = "pagestack::top")] fn top(&self) -> Arc { - let page = self.stack.borrow()[self.current()] - .as_ref() - .unwrap() - .clone(); - tracing::trace!(current = self.current(), page_id = page.get().id); + let current = self.current(); + let page = self.stack[current].clone().unwrap(); + tracing::trace!(current = current, page_id = page.get().id); turso_assert!(page.is_loaded(), "page should be loaded"); page } /// Current page pointer being used fn current(&self) -> usize { - let current = self.current_page.get() as usize; - assert!(self.current_page.get() >= 0); + assert!(self.current_page >= 0); + let current = self.current_page as usize; current } /// Cell index of the current page fn current_cell_index(&self) -> i32 { let current = self.current(); - self.node_states.borrow()[current].cell_idx + self.node_states[current].cell_idx } /// Check if the current cell index is less than 0. @@ -6038,58 +6025,53 @@ impl PageStack { /// Advance the current cell index of the current page to the next cell. /// We usually advance after going traversing a new page #[instrument(skip(self), level = Level::DEBUG, name = "pagestack::advance",)] - fn advance(&self) { + fn advance(&mut self) { let current = self.current(); tracing::trace!( - curr_cell_index = self.node_states.borrow()[current].cell_idx, - node_states = ?self.node_states.borrow().iter().map(|state| state.cell_idx).collect::>(), + curr_cell_index = self.node_states[current].cell_idx, + node_states = ?self.node_states.iter().map(|state| state.cell_idx).collect::>(), ); - self.node_states.borrow_mut()[current].cell_idx += 1; + self.node_states[current].cell_idx += 1; } #[instrument(skip(self), level = Level::DEBUG, name = "pagestack::retreat")] - fn retreat(&self) { + fn retreat(&mut self) { let current = self.current(); tracing::trace!( - curr_cell_index = self.node_states.borrow()[current].cell_idx, - node_states = ?self.node_states.borrow().iter().map(|state| state.cell_idx).collect::>(), + curr_cell_index = self.node_states[current].cell_idx, + node_states = ?self.node_states.iter().map(|state| state.cell_idx).collect::>(), ); - self.node_states.borrow_mut()[current].cell_idx -= 1; + self.node_states[current].cell_idx -= 1; } - fn set_cell_index(&self, idx: i32) { + fn set_cell_index(&mut self, idx: i32) { let current = self.current(); - self.node_states.borrow_mut()[current].cell_idx = idx; + self.node_states[current].cell_idx = idx; } fn has_parent(&self) -> bool { - self.current_page.get() > 0 + self.current_page > 0 } /// Get a page at a specific level in the stack (0 = root, 1 = first child, etc.) fn get_page_at_level(&self, level: usize) -> Option { - let stack = self.stack.borrow(); - if level < stack.len() { - stack[level].clone() + if level < self.stack.len() { + self.stack[level].clone() } else { None } } - fn unpin_all_if_pinned(&self) { - self.stack - .borrow_mut() - .iter_mut() - .flatten() - .for_each(|page| { - let _ = page.try_unpin(); - }); + fn unpin_all_if_pinned(&mut self) { + self.stack.iter_mut().flatten().for_each(|page| { + let _ = page.try_unpin(); + }); } - fn clear(&self) { + fn clear(&mut self) { self.unpin_all_if_pinned(); - self.current_page.set(-1); + self.current_page = -1; } } From db7c6b33709db75a66fc490724cfd5f9b07cee7e Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Tue, 2 Sep 2025 03:07:18 +0400 Subject: [PATCH 2/8] try to speed up count(*) where 1 = 1 --- core/storage/btree.rs | 53 ++++++++++++++----- core/types.rs | 115 ++++++++++++++++++++++-------------------- core/vdbe/execute.rs | 17 ++++--- core/vdbe/mod.rs | 22 ++++---- 4 files changed, 120 insertions(+), 87 deletions(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index 4a88a40ef..a019a9665 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -721,6 +721,7 @@ impl BTreeCursor { continue; } } + if cell_idx >= cell_count as i32 { self.stack.set_cell_index(cell_count as i32 - 1); } else if !self.stack.current_cell_index_less_than_min() { @@ -756,6 +757,7 @@ impl BTreeCursor { continue; } if contents.is_leaf() { + self.going_upwards = false; return Ok(IOResult::Done(true)); } @@ -1204,6 +1206,14 @@ impl BTreeCursor { } } loop { + let cell_idx = self.stack.current_cell_index(); + let cell_count = self.stack.leaf_cell_count(); + if cell_idx != -1 && cell_count.is_some() && cell_idx + 1 < cell_count.unwrap() { + self.stack.advance(); + return Ok(IOResult::Done(true)); + } + self.stack.set_leaf_cell_count(None); + let mem_page = self.stack.top(); let contents = mem_page.get_contents(); let cell_count = contents.cell_count(); @@ -1273,6 +1283,7 @@ impl BTreeCursor { ); if contents.is_leaf() { + self.stack.set_leaf_cell_count(Some(cell_count as i32)); return Ok(IOResult::Done(true)); } if is_index && self.going_upwards { @@ -4203,7 +4214,6 @@ impl BTreeCursor { let cursor_has_record = return_if_io!(self.get_next_record()); self.has_record.replace(cursor_has_record); self.invalidate_record(); - self.advance_state = AdvanceState::Start; return Ok(IOResult::Done(cursor_has_record)); } } @@ -4231,7 +4241,6 @@ impl BTreeCursor { let cursor_has_record = return_if_io!(self.get_prev_record()); self.has_record.replace(cursor_has_record); self.invalidate_record(); - self.advance_state = AdvanceState::Start; return Ok(IOResult::Done(cursor_has_record)); } } @@ -5148,7 +5157,8 @@ impl BTreeCursor { } fn get_immutable_record_or_create(&self) -> std::cell::RefMut<'_, Option> { - if self.reusable_immutable_record.borrow().is_none() { + let mut reusable_immutable_record = self.reusable_immutable_record.borrow_mut(); + if reusable_immutable_record.is_none() { let page_size = self .pager .page_size @@ -5156,9 +5166,9 @@ impl BTreeCursor { .expect("page size is not set") .get(); let record = ImmutableRecord::new(page_size as usize); - self.reusable_immutable_record.replace(Some(record)); + reusable_immutable_record.replace(record); } - self.reusable_immutable_record.borrow_mut() + reusable_immutable_record } fn get_immutable_record(&self) -> std::cell::RefMut<'_, Option> { @@ -5304,7 +5314,7 @@ impl BTreeCursor { self.context = None; self.valid_state = CursorValidState::Valid; return Ok(IOResult::Done(())); - } + }; let ctx = self.context.take().unwrap(); let seek_key = match ctx.key { CursorContextKey::TableRowId(rowid) => SeekKey::TableRowId(rowid), @@ -5985,9 +5995,12 @@ impl PageStack { page.unpin(); } + assert!(current > 0); self.node_states[current] = BTreeNodeState::default(); self.stack[current] = None; - assert!(current > 0); + // cell_count must be unset for last stack page by default + // (otherwise caller can think that he is at the leaf and enable hot-path optimization) + self.node_states[current - 1].cell_count = None; self.current_page -= 1; } @@ -6003,6 +6016,7 @@ impl PageStack { } /// Current page pointer being used + #[inline(always)] fn current(&self) -> usize { assert!(self.current_page >= 0); let current = self.current_page as usize; @@ -6015,6 +6029,20 @@ impl PageStack { self.node_states[current].cell_idx } + /// Cell count of the current leaf page + /// Caller must ensure that this method will be called for the leag page only + fn leaf_cell_count(&self) -> Option { + let current = self.current(); + self.node_states[current].cell_count + } + + // Set cell count for current leaf page + // Caller must ensure that this method will be called for the leag page only + fn set_leaf_cell_count(&mut self, cell_count: Option) { + let current = self.current(); + self.node_states[current].cell_count = cell_count; + } + /// Check if the current cell index is less than 0. /// This means we have been iterating backwards and have reached the start of the page. fn current_cell_index_less_than_min(&self) -> bool { @@ -6024,13 +6052,14 @@ impl PageStack { /// Advance the current cell index of the current page to the next cell. /// We usually advance after going traversing a new page - #[instrument(skip(self), level = Level::DEBUG, name = "pagestack::advance",)] + // #[instrument(skip(self), level = Level::DEBUG, name = "pagestack::advance",)] + #[inline(always)] fn advance(&mut self) { let current = self.current(); - tracing::trace!( - curr_cell_index = self.node_states[current].cell_idx, - node_states = ?self.node_states.iter().map(|state| state.cell_idx).collect::>(), - ); + // tracing::trace!( + // curr_cell_index = self.node_states[current].cell_idx, + // node_states = ?self.node_states.iter().map(|state| state.cell_idx).collect::>(), + // ); self.node_states[current].cell_idx += 1; } diff --git a/core/types.rs b/core/types.rs index 6bddb0b2b..b5d079ddb 100644 --- a/core/types.rs +++ b/core/types.rs @@ -759,83 +759,89 @@ impl Ord for Value { impl std::ops::Add for Value { type Output = Value; - fn add(self, rhs: Self) -> Self::Output { - match (self, rhs) { - (Self::Integer(int_left), Self::Integer(int_right)) => { - Self::Integer(int_left + int_right) - } - (Self::Integer(int_left), Self::Float(float_right)) => { - Self::Float(int_left as f64 + float_right) - } - (Self::Float(float_left), Self::Integer(int_right)) => { - Self::Float(float_left + int_right as f64) - } - (Self::Float(float_left), Self::Float(float_right)) => { - Self::Float(float_left + float_right) - } - (Self::Text(string_left), Self::Text(string_right)) => { - Self::build_text(&(string_left.as_str().to_string() + string_right.as_str())) - } - (Self::Text(string_left), Self::Integer(int_right)) => { - Self::build_text(&(string_left.as_str().to_string() + &int_right.to_string())) - } - (Self::Integer(int_left), Self::Text(string_right)) => { - Self::build_text(&(int_left.to_string() + string_right.as_str())) - } - (Self::Text(string_left), Self::Float(float_right)) => { - let string_right = Self::Float(float_right).to_string(); - Self::build_text(&(string_left.as_str().to_string() + &string_right)) - } - (Self::Float(float_left), Self::Text(string_right)) => { - let string_left = Self::Float(float_left).to_string(); - Self::build_text(&(string_left + string_right.as_str())) - } - (lhs, Self::Null) => lhs, - (Self::Null, rhs) => rhs, - _ => Self::Float(0.0), - } + fn add(mut self, rhs: Self) -> Self::Output { + self += rhs; + self } } impl std::ops::Add for Value { type Output = Value; - fn add(self, rhs: f64) -> Self::Output { - match self { - Self::Integer(int_left) => Self::Float(int_left as f64 + rhs), - Self::Float(float_left) => Self::Float(float_left + rhs), - _ => unreachable!(), - } + fn add(mut self, rhs: f64) -> Self::Output { + self += rhs; + self } } impl std::ops::Add for Value { type Output = Value; - fn add(self, rhs: i64) -> Self::Output { - match self { - Self::Integer(int_left) => Self::Integer(int_left + rhs), - Self::Float(float_left) => Self::Float(float_left + rhs as f64), - _ => unreachable!(), - } + fn add(mut self, rhs: i64) -> Self::Output { + self += rhs; + self } } impl std::ops::AddAssign for Value { - fn add_assign(&mut self, rhs: Self) { - *self = self.clone() + rhs; + fn add_assign(mut self: &mut Self, rhs: Self) { + match (&mut self, rhs) { + (Self::Integer(int_left), Self::Integer(int_right)) => *int_left += int_right, + (Self::Integer(int_left), Self::Float(float_right)) => { + *self = Self::Float(*int_left as f64 + float_right) + } + (Self::Float(float_left), Self::Integer(int_right)) => { + *self = Self::Float(*float_left + int_right as f64) + } + (Self::Float(float_left), Self::Float(float_right)) => { + *float_left += float_right; + } + (Self::Text(string_left), Self::Text(string_right)) => { + string_left.value.extend_from_slice(&string_right.value); + string_left.subtype = TextSubtype::Text; + } + (Self::Text(string_left), Self::Integer(int_right)) => { + let string_right = int_right.to_string(); + string_left.value.extend_from_slice(string_right.as_bytes()); + string_left.subtype = TextSubtype::Text; + } + (Self::Integer(int_left), Self::Text(string_right)) => { + let string_left = int_left.to_string(); + *self = Self::build_text(&(string_left + string_right.as_str())); + } + (Self::Text(string_left), Self::Float(float_right)) => { + let string_right = Self::Float(float_right).to_string(); + string_left.value.extend_from_slice(string_right.as_bytes()); + string_left.subtype = TextSubtype::Text; + } + (Self::Float(float_left), Self::Text(string_right)) => { + let string_left = Self::Float(*float_left).to_string(); + *self = Self::build_text(&(string_left + string_right.as_str())); + } + (_, Self::Null) => {} + (Self::Null, rhs) => *self = rhs, + _ => *self = Self::Float(0.0), + } } } impl std::ops::AddAssign for Value { fn add_assign(&mut self, rhs: i64) { - *self = self.clone() + rhs; + match self { + Self::Integer(int_left) => *int_left += rhs, + Self::Float(float_left) => *float_left += rhs as f64, + _ => unreachable!(), + } } } impl std::ops::AddAssign for Value { fn add_assign(&mut self, rhs: f64) { - *self = self.clone() + rhs; + match self { + Self::Integer(int_left) => *self = Self::Float(*int_left as f64 + rhs), + Self::Float(float_left) => *float_left += rhs, + _ => unreachable!(), + } } } @@ -2475,9 +2481,10 @@ impl IOResult { #[macro_export] macro_rules! return_if_io { ($expr:expr) => { - match $expr? { - IOResult::Done(v) => v, - IOResult::IO(io) => return Ok(IOResult::IO(io)), + match $expr { + Ok(IOResult::Done(v)) => v, + Ok(IOResult::IO(io)) => return Ok(IOResult::IO(io)), + Err(err) => return Err(err), } }; } diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 4cc20cc69..0b2347aa2 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -118,9 +118,10 @@ macro_rules! load_insn { macro_rules! return_if_io { ($expr:expr) => { - match $expr? { - IOResult::Done(v) => v, - IOResult::IO(io) => return Ok(InsnFunctionStepResult::IO(io)), + match $expr { + Ok(IOResult::Done(v)) => v, + Ok(IOResult::IO(io)) => return Ok(InsnFunctionStepResult::IO(io)), + Err(err) => return Err(err), } }; } @@ -3494,22 +3495,22 @@ pub fn op_agg_step( } } AggFunc::Count | AggFunc::Count0 => { - let col = state.registers[*col].get_value().clone(); + let skip = (matches!(func, AggFunc::Count) + && matches!(state.registers[*col].get_value(), Value::Null)); if matches!(&state.registers[*acc_reg], Register::Value(Value::Null)) { state.registers[*acc_reg] = Register::Aggregate(AggContext::Count(Value::Integer(0))); } - let Register::Aggregate(agg) = state.registers[*acc_reg].borrow_mut() else { + let Register::Aggregate(agg) = &mut state.registers[*acc_reg] else { panic!( "Unexpected value {:?} in AggStep at register {}", state.registers[*acc_reg], *acc_reg ); }; - let AggContext::Count(count) = agg.borrow_mut() else { + let AggContext::Count(count) = agg else { unreachable!(); }; - - if !(matches!(func, AggFunc::Count) && matches!(col, Value::Null)) { + if !skip { *count += 1; }; } diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index e581c9094..a38c25d96 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -415,15 +415,11 @@ impl Register { macro_rules! must_be_btree_cursor { ($cursor_id:expr, $cursor_ref:expr, $state:expr, $insn_name:expr) => {{ let (_, cursor_type) = $cursor_ref.get($cursor_id).unwrap(); - let cursor = match cursor_type { - CursorType::BTreeTable(_) => $crate::get_cursor!($state, $cursor_id), - CursorType::BTreeIndex(_) => $crate::get_cursor!($state, $cursor_id), - CursorType::MaterializedView(_, _) => $crate::get_cursor!($state, $cursor_id), - CursorType::Pseudo(_) => panic!("{} on pseudo cursor", $insn_name), - CursorType::Sorter => panic!("{} on sorter cursor", $insn_name), - CursorType::VirtualTable(_) => panic!("{} on virtual table cursor", $insn_name), - }; - cursor + if matches!(cursor_type, CursorType::BTreeTable(_) | CursorType::BTreeIndex(_)) | CursorType::Materialized(_, _) { + $state.get_cursor($cursor_id) + } else { + panic!("{} on unexpected cursor", $insn_name) + } }}; } @@ -471,6 +467,7 @@ impl Program { mv_store: Option>, pager: Rc, ) -> Result { + let enable_tracing = tracing::enabled!(tracing::Level::TRACE); loop { if self.connection.closed.get() { // Connection is closed for whatever reason, rollback the transaction. @@ -497,7 +494,9 @@ impl Program { // invalidate row let _ = state.result_row.take(); let (insn, insn_function) = &self.insns[state.pc as usize]; - trace_insn(self, state.pc as InsnReference, insn); + if enable_tracing { + trace_insn(self, state.pc as InsnReference, insn); + } // Always increment VM steps for every loop iteration state.metrics.vm_steps = state.metrics.vm_steps.saturating_add(1); @@ -832,9 +831,6 @@ pub fn registers_to_ref_values(registers: &[Register]) -> Vec { #[instrument(skip(program), level = Level::DEBUG)] fn trace_insn(program: &Program, addr: InsnReference, insn: &Insn) { - if !tracing::enabled!(tracing::Level::TRACE) { - return; - } tracing::trace!( "\n{}", explain::insn_to_str( From 9aed831f2f83e969a37dcf4eadd1cff354d3e5c5 Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Tue, 2 Sep 2025 13:58:32 +0400 Subject: [PATCH 3/8] format --- core/storage/btree.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index a019a9665..967cc6247 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -5938,7 +5938,7 @@ impl PageStack { // Pin the page to prevent it from being evicted while on the stack page.pin(); - self.stack[current] = Some(page.clone()); + self.stack[current] = Some(page); self.node_states[current] = BTreeNodeState { cell_idx: starting_cell_idx, cell_count: None, // we don't know the cell count yet, so we set it to None. any code pushing a child page onto the stack MUST set the parent page's cell_count. @@ -6019,8 +6019,7 @@ impl PageStack { #[inline(always)] fn current(&self) -> usize { assert!(self.current_page >= 0); - let current = self.current_page as usize; - current + self.current_page as usize } /// Cell index of the current page @@ -6052,14 +6051,9 @@ impl PageStack { /// Advance the current cell index of the current page to the next cell. /// We usually advance after going traversing a new page - // #[instrument(skip(self), level = Level::DEBUG, name = "pagestack::advance",)] #[inline(always)] fn advance(&mut self) { let current = self.current(); - // tracing::trace!( - // curr_cell_index = self.node_states[current].cell_idx, - // node_states = ?self.node_states.iter().map(|state| state.cell_idx).collect::>(), - // ); self.node_states[current].cell_idx += 1; } From 0b6a6e771310abbdbe677c224e58dd5313c882a6 Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Tue, 2 Sep 2025 14:04:36 +0400 Subject: [PATCH 4/8] remove comma --- core/storage/btree.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index 967cc6247..bf5d656d8 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -5314,7 +5314,7 @@ impl BTreeCursor { self.context = None; self.valid_state = CursorValidState::Valid; return Ok(IOResult::Done(())); - }; + } let ctx = self.context.take().unwrap(); let seek_key = match ctx.key { CursorContextKey::TableRowId(rowid) => SeekKey::TableRowId(rowid), From 5b9fe0cdf31a8eee2a724da0314645c8a64e1de9 Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Tue, 2 Sep 2025 15:41:48 +0400 Subject: [PATCH 5/8] fix --- core/storage/btree.rs | 41 ++++++++++++++++------------------------- 1 file changed, 16 insertions(+), 25 deletions(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index bf5d656d8..8892ac362 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -1206,17 +1206,18 @@ impl BTreeCursor { } } loop { + let mem_page = self.stack.top_ref(); + let contents = mem_page.get_contents(); let cell_idx = self.stack.current_cell_index(); - let cell_count = self.stack.leaf_cell_count(); - if cell_idx != -1 && cell_count.is_some() && cell_idx + 1 < cell_count.unwrap() { + let cell_count = contents.cell_count(); + let is_leaf = contents.is_leaf(); + if cell_idx != -1 && is_leaf && cell_idx as usize + 1 < cell_count { self.stack.advance(); return Ok(IOResult::Done(true)); } - self.stack.set_leaf_cell_count(None); - let mem_page = self.stack.top(); + let mem_page = mem_page.clone(); let contents = mem_page.get_contents(); - let cell_count = contents.cell_count(); tracing::debug!( id = mem_page.get().id, cell = self.stack.current_cell_index(), @@ -1274,16 +1275,15 @@ impl BTreeCursor { } turso_assert!( - cell_idx < contents.cell_count(), + cell_idx < cell_count, "cell index out of bounds: cell_idx={}, cell_count={}, page_type={:?} page_id={}", cell_idx, - contents.cell_count(), + cell_count, contents.page_type(), mem_page.get().id ); - if contents.is_leaf() { - self.stack.set_leaf_cell_count(Some(cell_count as i32)); + if is_leaf { return Ok(IOResult::Done(true)); } if is_index && self.going_upwards { @@ -6006,11 +6006,16 @@ impl PageStack { /// Get the top page on the stack. /// This is the page that is currently being traversed. - #[instrument(skip(self), level = Level::DEBUG, name = "pagestack::top")] fn top(&self) -> Arc { let current = self.current(); let page = self.stack[current].clone().unwrap(); - tracing::trace!(current = current, page_id = page.get().id); + turso_assert!(page.is_loaded(), "page should be loaded"); + page + } + + fn top_ref(&self) -> &Arc { + let current = self.current(); + let page = self.stack[current].as_ref().unwrap(); turso_assert!(page.is_loaded(), "page should be loaded"); page } @@ -6028,20 +6033,6 @@ impl PageStack { self.node_states[current].cell_idx } - /// Cell count of the current leaf page - /// Caller must ensure that this method will be called for the leag page only - fn leaf_cell_count(&self) -> Option { - let current = self.current(); - self.node_states[current].cell_count - } - - // Set cell count for current leaf page - // Caller must ensure that this method will be called for the leag page only - fn set_leaf_cell_count(&mut self, cell_count: Option) { - let current = self.current(); - self.node_states[current].cell_count = cell_count; - } - /// Check if the current cell index is less than 0. /// This means we have been iterating backwards and have reached the start of the page. fn current_cell_index_less_than_min(&self) -> bool { From 47808e9da8d9bbf08298425d4f815e30fa98993f Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Tue, 2 Sep 2025 15:43:21 +0400 Subject: [PATCH 6/8] enable tracing subscriber in integration tests --- tests/integration/common.rs | 3 --- tests/integration/functions/mod.rs | 13 ------------- tests/integration/mod.rs | 13 +++++++++++++ 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/tests/integration/common.rs b/tests/integration/common.rs index 48ea009dd..a203c29f0 100644 --- a/tests/integration/common.rs +++ b/tests/integration/common.rs @@ -64,9 +64,6 @@ impl TempDatabase { } pub fn new_with_rusqlite(table_sql: &str, enable_indexes: bool) -> Self { - let _ = tracing_subscriber::fmt() - .with_max_level(tracing::Level::TRACE) - .finish(); let mut path = TempDir::new().unwrap().keep(); path.push("test.db"); { diff --git a/tests/integration/functions/mod.rs b/tests/integration/functions/mod.rs index 8ee95c32d..b31ed8f53 100644 --- a/tests/integration/functions/mod.rs +++ b/tests/integration/functions/mod.rs @@ -1,16 +1,3 @@ mod test_cdc; mod test_function_rowid; mod test_wal_api; - -#[cfg(test)] -mod tests { - use tracing_subscriber::EnvFilter; - - #[ctor::ctor] - fn init() { - tracing_subscriber::fmt() - .with_env_filter(EnvFilter::from_default_env()) - .with_ansi(false) - .init(); - } -} diff --git a/tests/integration/mod.rs b/tests/integration/mod.rs index 0f45007c6..42d782d5b 100644 --- a/tests/integration/mod.rs +++ b/tests/integration/mod.rs @@ -5,3 +5,16 @@ mod fuzz_transaction; mod pragma; mod query_processing; mod wal; + +#[cfg(test)] +mod tests { + use tracing_subscriber::EnvFilter; + + #[ctor::ctor] + fn init() { + tracing_subscriber::fmt() + .with_env_filter(EnvFilter::from_default_env()) + .with_ansi(false) + .init(); + } +} From cd627c2368879df58eca1a2b565d34d2cdc13a34 Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Tue, 2 Sep 2025 15:59:51 +0400 Subject: [PATCH 7/8] remove unnecessary changes --- core/storage/btree.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/core/storage/btree.rs b/core/storage/btree.rs index 8892ac362..8b37dc64b 100644 --- a/core/storage/btree.rs +++ b/core/storage/btree.rs @@ -757,7 +757,6 @@ impl BTreeCursor { continue; } if contents.is_leaf() { - self.going_upwards = false; return Ok(IOResult::Done(true)); } @@ -5998,9 +5997,6 @@ impl PageStack { assert!(current > 0); self.node_states[current] = BTreeNodeState::default(); self.stack[current] = None; - // cell_count must be unset for last stack page by default - // (otherwise caller can think that he is at the leaf and enable hot-path optimization) - self.node_states[current - 1].cell_count = None; self.current_page -= 1; } From 87d49cd039c7953b16a1af8aed0ccbe4f3ef9bd5 Mon Sep 17 00:00:00 2001 From: Nikita Sivukhin Date: Sun, 7 Sep 2025 19:56:17 +0400 Subject: [PATCH 8/8] cargo fmt after rebase --- core/vdbe/mod.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs index a38c25d96..c5e4e91a8 100644 --- a/core/vdbe/mod.rs +++ b/core/vdbe/mod.rs @@ -415,8 +415,13 @@ impl Register { macro_rules! must_be_btree_cursor { ($cursor_id:expr, $cursor_ref:expr, $state:expr, $insn_name:expr) => {{ let (_, cursor_type) = $cursor_ref.get($cursor_id).unwrap(); - if matches!(cursor_type, CursorType::BTreeTable(_) | CursorType::BTreeIndex(_)) | CursorType::Materialized(_, _) { - $state.get_cursor($cursor_id) + if matches!( + cursor_type, + CursorType::BTreeTable(_) + | CursorType::BTreeIndex(_) + | CursorType::MaterializedView(_, _) + ) { + $crate::get_cursor!($state, $cursor_id) } else { panic!("{} on unexpected cursor", $insn_name) }