From b14124ad3b7068bea9a4dd23f4477f56195d0834 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Mon, 21 Jul 2025 15:00:14 +0300 Subject: [PATCH 1/5] VDBE/op_column: avoid first cloning text/blob and then copying it again instead use RefValue to refer to record payload directly and then copy to register as necessary --- core/vdbe/execute.rs | 111 ++++++++++++++++++++++++++++++++----------- 1 file changed, 83 insertions(+), 28 deletions(-) diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 2ff358568..42af7b710 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -10,7 +10,8 @@ use crate::storage::wal::DummyWAL; use crate::storage::{self, header_accessor}; use crate::translate::collate::CollationSeq; use crate::types::{ - compare_immutable, compare_records_generic, ImmutableRecord, SeekResult, Text, TextSubtype, + compare_immutable, compare_records_generic, ImmutableRecord, RawSlice, SeekResult, Text, + TextRef, TextSubtype, }; use crate::util::normalize_ident; use crate::vdbe::insn::InsertFlags; @@ -1420,18 +1421,18 @@ pub fn op_column( let cursor = cursor.as_btree_mut(); if cursor.get_null_flag() { - break 'value Value::Null; + break 'value Some(RefValue::Null); } let record_result = return_if_io!(cursor.record()); let Some(record) = record_result.as_ref() else { - break 'value default.clone().unwrap_or(Value::Null); + break 'value None; }; let payload = record.get_payload(); if payload.is_empty() { - break 'value default.clone().unwrap_or(Value::Null); + break 'value None; } let mut record_cursor = cursor.record_cursor.borrow_mut(); @@ -1507,25 +1508,25 @@ pub fn op_column( record_cursor.offsets.clear(); record_cursor.header_offset = 0; record_cursor.header_size = 0; - break 'value default.clone().unwrap_or(Value::Null); + break 'value None; } if target_column >= record_cursor.serial_types.len() { - break 'value default.clone().unwrap_or(Value::Null); + break 'value None; } let serial_type = record_cursor.serial_types[target_column]; // Fast path for common constant cases match serial_type { - 0 => break 'value Value::Null, - 8 => break 'value Value::Integer(0), - 9 => break 'value Value::Integer(1), + 0 => break 'value Some(RefValue::Null), + 8 => break 'value Some(RefValue::Integer(0)), + 9 => break 'value Some(RefValue::Integer(1)), _ => {} } if target_column + 1 >= record_cursor.offsets.len() { - break 'value default.clone().unwrap_or(Value::Null); + break 'value None; } let start_offset = record_cursor.offsets[target_column]; @@ -1547,7 +1548,10 @@ pub fn op_column( }; if data_len >= expected_len { - Value::Integer(read_integer_fast(data_slice, expected_len)) + Some(RefValue::Integer(read_integer_fast( + data_slice, + expected_len, + ))) } else { return Err(LimboError::Corrupt(format!( "Insufficient data for integer type {serial_type}: expected {expected_len}, got {data_len}" @@ -1566,41 +1570,92 @@ pub fn op_column( data_slice[6], data_slice[7], ]; - Value::Float(f64::from_be_bytes(bytes)) + Some(RefValue::Float(f64::from_be_bytes(bytes))) } else { - default.clone().unwrap_or(Value::Null) + None } } - n if n >= 12 && n % 2 == 0 => Value::Blob(data_slice.to_vec()), - n if n >= 13 && n % 2 == 1 => Value::Text(Text { - value: data_slice.to_vec(), - subtype: TextSubtype::Text, - }), - _ => default.clone().unwrap_or(Value::Null), + n if n >= 12 && n % 2 == 0 => { + Some(RefValue::Blob(RawSlice::create_from(data_slice))) + } + n if n >= 13 && n % 2 == 1 => Some(RefValue::Text(TextRef::create_from( + data_slice, + TextSubtype::Text, + ))), + _ => None, } }; + let Some(value) = value else { + // DEFAULT handling. Try to reuse the registers when allocation is not needed. + let Some(ref default) = default else { + state.registers[*dest] = Register::Value(Value::Null); + state.pc += 1; + return Ok(InsnFunctionStepResult::Step); + }; + match (default, &mut state.registers[*dest]) { + (Value::Text(new_text), Register::Value(Value::Text(existing_text))) => { + let new_text_str = new_text.as_str(); + if existing_text.value.capacity() >= new_text_str.len() { + existing_text.value.clear(); + existing_text + .value + .extend_from_slice(new_text_str.as_bytes()); + existing_text.subtype = new_text.subtype; + } else { + state.registers[*dest] = + Register::Value(Value::Text(Text::new(new_text_str))); + } + } + (Value::Blob(new_blob), Register::Value(Value::Blob(existing_blob))) => { + if existing_blob.capacity() >= new_blob.len() { + existing_blob.clear(); + existing_blob.extend_from_slice(new_blob); + } else { + state.registers[*dest] = + Register::Value(Value::Blob(new_blob.to_vec())); + } + } + _ => { + state.registers[*dest] = Register::Value(default.clone()); + } + } + return Ok(InsnFunctionStepResult::Step); + }; + // Try to reuse the registers when allocation is not needed. match (&value, &mut state.registers[*dest]) { - (Value::Text(new_text), Register::Value(Value::Text(existing_text))) => { - if existing_text.value.capacity() >= new_text.value.len() { + (RefValue::Text(new_text), Register::Value(Value::Text(existing_text))) => { + let new_text_str = new_text.as_str(); + if existing_text.value.capacity() >= new_text_str.len() { existing_text.value.clear(); - existing_text.value.extend_from_slice(&new_text.value); + existing_text + .value + .extend_from_slice(new_text_str.as_bytes()); existing_text.subtype = new_text.subtype; } else { - state.registers[*dest] = Register::Value(value); + state.registers[*dest] = + Register::Value(Value::Text(Text::new(new_text_str))); } } - (Value::Blob(new_blob), Register::Value(Value::Blob(existing_blob))) => { - if existing_blob.capacity() >= new_blob.len() { + (RefValue::Blob(new_blob), Register::Value(Value::Blob(existing_blob))) => { + let new_blob_slice = new_blob.to_slice(); + if existing_blob.capacity() >= new_blob_slice.len() { existing_blob.clear(); - existing_blob.extend_from_slice(new_blob); + existing_blob.extend_from_slice(new_blob_slice); } else { - state.registers[*dest] = Register::Value(value); + state.registers[*dest] = + Register::Value(Value::Blob(new_blob_slice.to_vec())); } } _ => { - state.registers[*dest] = Register::Value(value); + state.registers[*dest] = Register::Value(match value { + RefValue::Integer(i) => Value::Integer(i), + RefValue::Float(f) => Value::Float(f), + RefValue::Text(t) => Value::Text(Text::new(t.as_str())), + RefValue::Blob(b) => Value::Blob(b.to_slice().to_vec()), + RefValue::Null => Value::Null, + }); } } } From 7eb52c65d309897ff388b1b47b9f3a2781928d62 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Thu, 24 Jul 2025 10:22:26 +0300 Subject: [PATCH 2/5] Add missing program counter increment --- core/vdbe/execute.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 42af7b710..98de72e67 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -1620,6 +1620,7 @@ pub fn op_column( state.registers[*dest] = Register::Value(default.clone()); } } + state.pc += 1; return Ok(InsnFunctionStepResult::Step); }; From 12d8b266a11108bf5d7408b25772292b3d294167 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Thu, 24 Jul 2025 10:37:27 +0300 Subject: [PATCH 3/5] Define some helper traits to reduce duplication --- core/types.rs | 68 +++++++++++++++++++++++++++++++++++++++++++- core/vdbe/execute.rs | 39 ++++++------------------- 2 files changed, 76 insertions(+), 31 deletions(-) diff --git a/core/types.rs b/core/types.rs index b1cf976cf..acfa4ccd0 100644 --- a/core/types.rs +++ b/core/types.rs @@ -82,7 +82,6 @@ impl Text { subtype: TextSubtype::Text, } } - #[cfg(feature = "json")] pub fn json(value: String) -> Self { Self { @@ -96,6 +95,73 @@ impl Text { } } +pub trait Extendable { + fn maybe_extend(&mut self, other: &T) -> bool; +} + +impl Extendable for Text { + fn maybe_extend(&mut self, other: &T) -> bool { + if self.value.capacity() >= other.as_ref().len() { + self.value.clear(); + self.value.extend_from_slice(other.as_ref().as_bytes()); + self.subtype = other.subtype(); + true + } else { + false + } + } +} + +impl Extendable for Vec { + fn maybe_extend(&mut self, other: &T) -> bool { + if self.capacity() >= other.as_slice().len() { + self.clear(); + self.extend_from_slice(other.as_slice()); + true + } else { + false + } + } +} + +pub trait AnyText: AsRef { + fn subtype(&self) -> TextSubtype; +} + +impl AsRef for TextRef { + fn as_ref(&self) -> &str { + self.as_str() + } +} + +impl AnyText for Text { + fn subtype(&self) -> TextSubtype { + self.subtype + } +} + +impl AnyText for TextRef { + fn subtype(&self) -> TextSubtype { + self.subtype + } +} + +pub trait AnyBlob { + fn as_slice(&self) -> &[u8]; +} + +impl AnyBlob for RawSlice { + fn as_slice(&self) -> &[u8] { + self.to_slice() + } +} + +impl AnyBlob for Vec { + fn as_slice(&self) -> &[u8] { + self.as_slice() + } +} + impl AsRef for Text { fn as_ref(&self) -> &str { self.as_str() diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 98de72e67..2bd73263b 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -10,8 +10,8 @@ use crate::storage::wal::DummyWAL; use crate::storage::{self, header_accessor}; use crate::translate::collate::CollationSeq; use crate::types::{ - compare_immutable, compare_records_generic, ImmutableRecord, RawSlice, SeekResult, Text, - TextRef, TextSubtype, + compare_immutable, compare_records_generic, Extendable, ImmutableRecord, RawSlice, SeekResult, + Text, TextRef, TextSubtype, }; use crate::util::normalize_ident; use crate::vdbe::insn::InsertFlags; @@ -1595,23 +1595,13 @@ pub fn op_column( }; match (default, &mut state.registers[*dest]) { (Value::Text(new_text), Register::Value(Value::Text(existing_text))) => { - let new_text_str = new_text.as_str(); - if existing_text.value.capacity() >= new_text_str.len() { - existing_text.value.clear(); - existing_text - .value - .extend_from_slice(new_text_str.as_bytes()); - existing_text.subtype = new_text.subtype; - } else { + if !existing_text.maybe_extend(new_text) { state.registers[*dest] = - Register::Value(Value::Text(Text::new(new_text_str))); + Register::Value(Value::Text(Text::new(new_text.as_str()))); } } (Value::Blob(new_blob), Register::Value(Value::Blob(existing_blob))) => { - if existing_blob.capacity() >= new_blob.len() { - existing_blob.clear(); - existing_blob.extend_from_slice(new_blob); - } else { + if !existing_blob.maybe_extend(new_blob) { state.registers[*dest] = Register::Value(Value::Blob(new_blob.to_vec())); } @@ -1627,26 +1617,15 @@ pub fn op_column( // Try to reuse the registers when allocation is not needed. match (&value, &mut state.registers[*dest]) { (RefValue::Text(new_text), Register::Value(Value::Text(existing_text))) => { - let new_text_str = new_text.as_str(); - if existing_text.value.capacity() >= new_text_str.len() { - existing_text.value.clear(); - existing_text - .value - .extend_from_slice(new_text_str.as_bytes()); - existing_text.subtype = new_text.subtype; - } else { + if !existing_text.maybe_extend(new_text) { state.registers[*dest] = - Register::Value(Value::Text(Text::new(new_text_str))); + Register::Value(Value::Text(Text::new(new_text.as_str()))); } } (RefValue::Blob(new_blob), Register::Value(Value::Blob(existing_blob))) => { - let new_blob_slice = new_blob.to_slice(); - if existing_blob.capacity() >= new_blob_slice.len() { - existing_blob.clear(); - existing_blob.extend_from_slice(new_blob_slice); - } else { + if !existing_blob.maybe_extend(new_blob) { state.registers[*dest] = - Register::Value(Value::Blob(new_blob_slice.to_vec())); + Register::Value(Value::Blob(new_blob.to_slice().to_vec())); } } _ => { From ae5470f1d057760a3755421f76de77208966c959 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Thu, 24 Jul 2025 14:12:35 +0300 Subject: [PATCH 4/5] use default directly --- core/vdbe/execute.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 2bd73263b..137b925c6 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -1596,14 +1596,12 @@ pub fn op_column( match (default, &mut state.registers[*dest]) { (Value::Text(new_text), Register::Value(Value::Text(existing_text))) => { if !existing_text.maybe_extend(new_text) { - state.registers[*dest] = - Register::Value(Value::Text(Text::new(new_text.as_str()))); + state.registers[*dest] = Register::Value(default.clone()); } } (Value::Blob(new_blob), Register::Value(Value::Blob(existing_blob))) => { if !existing_blob.maybe_extend(new_blob) { - state.registers[*dest] = - Register::Value(Value::Blob(new_blob.to_vec())); + state.registers[*dest] = Register::Value(default.clone()); } } _ => { From 111c0032ae8c7e29d6bd235a35c9096137f0a917 Mon Sep 17 00:00:00 2001 From: Jussi Saurio Date: Mon, 28 Jul 2025 11:06:16 +0300 Subject: [PATCH 5/5] Always extend texts and blobs --- core/types.rs | 26 ++++++++------------------ core/vdbe/execute.rs | 18 ++++-------------- 2 files changed, 12 insertions(+), 32 deletions(-) diff --git a/core/types.rs b/core/types.rs index acfa4ccd0..49267cf38 100644 --- a/core/types.rs +++ b/core/types.rs @@ -96,31 +96,21 @@ impl Text { } pub trait Extendable { - fn maybe_extend(&mut self, other: &T) -> bool; + fn do_extend(&mut self, other: &T); } impl Extendable for Text { - fn maybe_extend(&mut self, other: &T) -> bool { - if self.value.capacity() >= other.as_ref().len() { - self.value.clear(); - self.value.extend_from_slice(other.as_ref().as_bytes()); - self.subtype = other.subtype(); - true - } else { - false - } + fn do_extend(&mut self, other: &T) { + self.value.clear(); + self.value.extend_from_slice(other.as_ref().as_bytes()); + self.subtype = other.subtype(); } } impl Extendable for Vec { - fn maybe_extend(&mut self, other: &T) -> bool { - if self.capacity() >= other.as_slice().len() { - self.clear(); - self.extend_from_slice(other.as_slice()); - true - } else { - false - } + fn do_extend(&mut self, other: &T) { + self.clear(); + self.extend_from_slice(other.as_slice()); } } diff --git a/core/vdbe/execute.rs b/core/vdbe/execute.rs index 137b925c6..af34c58a4 100644 --- a/core/vdbe/execute.rs +++ b/core/vdbe/execute.rs @@ -1595,14 +1595,10 @@ pub fn op_column( }; match (default, &mut state.registers[*dest]) { (Value::Text(new_text), Register::Value(Value::Text(existing_text))) => { - if !existing_text.maybe_extend(new_text) { - state.registers[*dest] = Register::Value(default.clone()); - } + existing_text.do_extend(new_text); } (Value::Blob(new_blob), Register::Value(Value::Blob(existing_blob))) => { - if !existing_blob.maybe_extend(new_blob) { - state.registers[*dest] = Register::Value(default.clone()); - } + existing_blob.do_extend(new_blob); } _ => { state.registers[*dest] = Register::Value(default.clone()); @@ -1615,16 +1611,10 @@ pub fn op_column( // Try to reuse the registers when allocation is not needed. match (&value, &mut state.registers[*dest]) { (RefValue::Text(new_text), Register::Value(Value::Text(existing_text))) => { - if !existing_text.maybe_extend(new_text) { - state.registers[*dest] = - Register::Value(Value::Text(Text::new(new_text.as_str()))); - } + existing_text.do_extend(new_text); } (RefValue::Blob(new_blob), Register::Value(Value::Blob(existing_blob))) => { - if !existing_blob.maybe_extend(new_blob) { - state.registers[*dest] = - Register::Value(Value::Blob(new_blob.to_slice().to_vec())); - } + existing_blob.do_extend(new_blob); } _ => { state.registers[*dest] = Register::Value(match value {