Repository: tursodatabase/limbo (language: rust)

Issue:
Don't allocate comments while translating statement unless query is an EXPLAIN
currently we have a `comments` hashmap in `ProgramBuilder` that stores `&'static str` comments that are only used for `EXPLAIN`. This is needless work for any query that is not an EXPLAIN


Discussion / hints:

Fix patch:
diff --git a/core/lib.rs b/core/lib.rs
index 0addaffc5..bcb87d6fd 100644
--- a/core/lib.rs
+++ b/core/lib.rs
@@ -42,6 +42,7 @@ pub use storage::wal::WalFile;
 pub use storage::wal::WalFileShared;
 pub use types::Value;
 use util::parse_schema_rows;
+use vdbe::builder::QueryMode;
 
 pub use error::LimboError;
 use translate::select::prepare_select_plan;
@@ -273,6 +274,7 @@ impl Connection {
                         self.pager.clone(),
                         Rc::downgrade(self),
                         syms,
+                        QueryMode::Normal,
                     )?);
                     Ok(Statement::new(program, self.pager.clone()))
                 }
@@ -307,6 +309,7 @@ impl Connection {
                     self.pager.clone(),
                     Rc::downgrade(self),
                     syms,
+                    QueryMode::Normal,
                 )?);
                 let stmt = Statement::new(program, self.pager.clone());
                 Ok(Some(stmt))
@@ -319,6 +322,7 @@ impl Connection {
                     self.pager.clone(),
                     Rc::downgrade(self),
                     syms,
+                    QueryMode::Explain,
                 )?;
                 program.explain();
                 Ok(None)
@@ -361,6 +365,7 @@ impl Connection {
                         self.pager.clone(),
                         Rc::downgrade(self),
                         syms,
+                        QueryMode::Explain,
                     )?;
                     program.explain();
                 }
@@ -373,6 +378,7 @@ impl Connection {
                         self.pager.clone(),
                         Rc::downgrade(self),
                         syms,
+                        QueryMode::Normal,
                     )?;
 
                     let mut state =
diff --git a/core/translate/mod.rs b/core/translate/mod.rs
index 8a046b9b2..82b115c74 100644
--- a/core/translate/mod.rs
+++ b/core/translate/mod.rs
@@ -28,7 +28,7 @@ use crate::storage::pager::Pager;
 use crate::storage::sqlite3_ondisk::DatabaseHeader;
 use crate::translate::delete::translate_delete;
 use crate::util::PRIMARY_KEY_AUTOMATIC_INDEX_NAME_PREFIX;
-use crate::vdbe::builder::CursorType;
+use crate::vdbe::builder::{CursorType, QueryMode};
 use crate::vdbe::{builder::ProgramBuilder, insn::Insn, Program};
 use crate::{bail_parse_error, Connection, LimboError, Result, SymbolTable};
 use insert::translate_insert;
@@ -46,8 +46,9 @@ pub fn translate(
     pager: Rc<Pager>,
     connection: Weak<Connection>,
     syms: &SymbolTable,
+    query_mode: QueryMode,
 ) -> Result<Program> {
-    let mut program = ProgramBuilder::new();
+    let mut program = ProgramBuilder::new(query_mode);
     let mut change_cnt_on = false;
 
     match stmt {
diff --git a/core/vdbe/builder.rs b/core/vdbe/builder.rs
index 1752e9dd3..7ad71fed4 100644
--- a/core/vdbe/builder.rs
+++ b/core/vdbe/builder.rs
@@ -27,8 +27,8 @@ pub struct ProgramBuilder {
     label_to_resolved_offset: HashMap<i32, u32>,
     // Bitmask of cursors that have emitted a SeekRowid instruction.
     seekrowid_emitted_bitmask: u64,
-    // map of instruction index to manual comment (used in EXPLAIN)
-    comments: HashMap<InsnReference, &'static str>,
+    // map of instruction index to manual comment (used in EXPLAIN only)
+    comments: Option<HashMap<InsnReference, &'static str>>,
     pub parameters: Parameters,
     pub columns: Vec<String>,
 }
@@ -47,8 +47,14 @@ impl CursorType {
     }
 }
 
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum QueryMode {
+    Normal,
+    Explain,
+}
+
 impl ProgramBuilder {
-    pub fn new() -> Self {
+    pub fn new(query_mode: QueryMode) -> Self {
         Self {
             next_free_register: 1,
             next_free_label: 0,
@@ -59,7 +65,11 @@ impl ProgramBuilder {
             constant_insns: Vec::new(),
             label_to_resolved_offset: HashMap::new(),
             seekrowid_emitted_bitmask: 0,
-            comments: HashMap::new(),
+            comments: if query_mode == QueryMode::Explain {
+                Some(HashMap::new())
+            } else {
+                None
+            },
             parameters: Parameters::new(),
             columns: Vec::new(),
         }
@@ -163,7 +173,9 @@ impl ProgramBuilder {
     }
 
     pub fn add_comment(&mut self, insn_index: BranchOffset, comment: &'static str) {
-        self.comments.insert(insn_index.to_offset_int(), comment);
+        if let Some(comments) = &mut self.comments {
+            comments.insert(insn_index.to_offset_int(), comment);
+        }
     }
 
     // Emit an instruction that will be put at the end of the program (after Transaction statement).
diff --git a/core/vdbe/mod.rs b/core/vdbe/mod.rs
index 16156347a..0c8d783e5 100644
--- a/core/vdbe/mod.rs
+++ b/core/vdbe/mod.rs
@@ -377,7 +377,7 @@ pub struct Program {
     pub insns: Vec<Insn>,
     pub cursor_ref: Vec<(Option<String>, CursorType)>,
     pub database_header: Rc<RefCell<DatabaseHeader>>,
-    pub comments: HashMap<InsnReference, &'static str>,
+    pub comments: Option<HashMap<InsnReference, &'static str>>,
     pub parameters: crate::parameters::Parameters,
     pub connection: Weak<Connection>,
     pub auto_commit: bool,
@@ -2551,7 +2551,10 @@ fn trace_insn(program: &Program, addr: InsnReference, insn: &Insn) {
             addr,
             insn,
             String::new(),
-            program.comments.get(&{ addr }).copied()
+            program
+                .comments
+                .as_ref()
+                .and_then(|comments| comments.get(&{ addr }).copied())
         )
     );
 }
@@ -2562,7 +2565,10 @@ fn print_insn(program: &Program, addr: InsnReference, insn: &Insn, indent: Strin
         addr,
         insn,
         indent,
-        program.comments.get(&{ addr }).copied(),
+        program
+            .comments
+            .as_ref()
+            .and_then(|comments| comments.get(&{ addr }).copied()),
     );
     println!("{}", s);
 }


Test patch: