From c576acaf109615e6c0f3d1e605925c4dc0728774 Mon Sep 17 00:00:00 2001
From: Denis Smirnov <sd@picodata.io>
Date: Thu, 18 Aug 2022 16:48:44 +0700
Subject: [PATCH] perf: remove redundant virtual table clone
---
Cargo.toml | 2 +-
src/executor.rs | 3 ++-
.../engine/cartridge/backend/sql/tree.rs | 23 +++++++++++--------
src/executor/ir.rs | 16 ++++++++-----
4 files changed, 26 insertions(+), 18 deletions(-)
diff --git a/Cargo.toml b/Cargo.toml
index 0170ed12a8..e1576b630d 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -16,7 +16,7 @@ itertools = "0.10"
pest = "2.0"
pest_derive = "2.0"
rand = "0.8"
-serde = { version = "1.0", features = ["derive"] }
+serde = { version = "1.0", features = ["derive", "rc"] }
serde_yaml = "0.8"
sha2 = "0.10"
tarantool = { git = "https://git.picodata.io/picodata/picodata/tarantool-module.git", features = ["picodata", "schema"] }
diff --git a/src/executor.rs b/src/executor.rs
index bfc38102e6..db2e3867ac 100644
--- a/src/executor.rs
+++ b/src/executor.rs
@@ -26,6 +26,7 @@
use ahash::RandomState;
use std::any::Any;
use std::collections::{hash_map::Entry, HashMap};
+use std::rc::Rc;
use crate::errors::QueryPlannerError;
use crate::executor::bucket::Buckets;
@@ -200,7 +201,7 @@ where
}
if let Some(vtables) = self.exec_plan.get_mut_vtables() {
- vtables.insert(motion_id, vtable);
+ vtables.insert(motion_id, Rc::new(vtable));
}
Ok(())
diff --git a/src/executor/engine/cartridge/backend/sql/tree.rs b/src/executor/engine/cartridge/backend/sql/tree.rs
index 142f3ac5ff..4590883c56 100644
--- a/src/executor/engine/cartridge/backend/sql/tree.rs
+++ b/src/executor/engine/cartridge/backend/sql/tree.rs
@@ -1,5 +1,6 @@
use ahash::RandomState;
use std::collections::HashMap;
+use std::rc::Rc;
use serde::{Deserialize, Serialize};
use traversal::DftPost;
@@ -12,7 +13,7 @@ use crate::ir::operator::{Bool, Relational};
use crate::ir::Node;
/// Payload of the syntax tree node.
-#[derive(Serialize, Deserialize, PartialEq, Debug, Clone)]
+#[derive(Deserialize, Serialize, PartialEq, Debug, Clone)]
pub enum SyntaxData {
/// "as alias_name"
Alias(String),
@@ -33,11 +34,11 @@ pub enum SyntaxData {
/// parameter (a wrapper over a plan constants)
Parameter(usize),
/// virtual table
- VTable(Box<VirtualTable>),
+ VTable(Rc<VirtualTable>),
}
/// A syntax tree node.
-#[derive(Serialize, Deserialize, PartialEq, Debug, Clone)]
+#[derive(Deserialize, Serialize, PartialEq, Debug, Clone)]
pub struct SyntaxNode {
/// Payload
pub(crate) data: SyntaxData,
@@ -133,7 +134,7 @@ impl SyntaxNode {
}
}
- fn new_vtable(value: Box<VirtualTable>) -> Self {
+ fn new_vtable(value: Rc<VirtualTable>) -> Self {
SyntaxNode {
data: SyntaxData::VTable(value),
left: None,
@@ -143,7 +144,7 @@ impl SyntaxNode {
}
/// Storage for the syntax nodes.
-#[derive(Serialize, Deserialize, PartialEq, Debug)]
+#[derive(Deserialize, Serialize, PartialEq, Debug)]
pub struct SyntaxNodes {
pub(crate) arena: Vec<SyntaxNode>,
map: HashMap<usize, usize, RandomState>,
@@ -573,7 +574,7 @@ impl<'p> SyntaxPlan<'p> {
Ok(self.nodes.push_syntax_node(sn))
}
Relational::Motion { .. } => {
- let vtable = self.plan.get_motion_vtable(id)?.clone();
+ let vtable = self.plan.get_motion_vtable(id)?;
let vtable_alias = vtable.get_alias().map(String::from);
let child_id = self.plan.get_motion_child(id)?;
let child_rel = self.plan.get_ir_plan().get_relation_node(child_id)?;
@@ -582,7 +583,7 @@ impl<'p> SyntaxPlan<'p> {
children = Vec::from([
self.nodes.push_syntax_node(SyntaxNode::new_open()),
self.nodes
- .push_syntax_node(SyntaxNode::new_vtable(Box::new(vtable))),
+ .push_syntax_node(SyntaxNode::new_vtable(Rc::clone(&vtable))),
self.nodes.push_syntax_node(SyntaxNode::new_close()),
]);
@@ -592,7 +593,7 @@ impl<'p> SyntaxPlan<'p> {
} else {
children.push(
self.nodes
- .push_syntax_node(SyntaxNode::new_vtable(Box::new(vtable))),
+ .push_syntax_node(SyntaxNode::new_vtable(Rc::clone(&vtable))),
);
}
@@ -660,7 +661,7 @@ impl<'p> SyntaxPlan<'p> {
if let Some(motion_id) = ir_plan.get_motion_among_rel_nodes(&rel_ids)? {
// Replace motion node to virtual table node
- let vtable = self.plan.get_motion_vtable(motion_id)?.clone();
+ let vtable = self.plan.get_motion_vtable(motion_id)?;
if vtable.get_alias().is_none() {
let sn = SyntaxNode::new_pointer(
id,
@@ -668,7 +669,9 @@ impl<'p> SyntaxPlan<'p> {
vec![
self.nodes.push_syntax_node(SyntaxNode::new_open()),
self.nodes
- .push_syntax_node(SyntaxNode::new_vtable(Box::new(vtable))),
+ .push_syntax_node(SyntaxNode::new_vtable(Rc::clone(
+ &vtable,
+ ))),
self.nodes.push_syntax_node(SyntaxNode::new_close()),
],
);
diff --git a/src/executor/ir.rs b/src/executor/ir.rs
index 6d026a7b7a..cc21dbf7bd 100644
--- a/src/executor/ir.rs
+++ b/src/executor/ir.rs
@@ -1,4 +1,5 @@
use std::collections::HashMap;
+use std::rc::Rc;
use crate::errors::QueryPlannerError;
use crate::errors::QueryPlannerError::CustomError;
@@ -10,7 +11,7 @@ use crate::ir::Plan;
#[derive(Debug, Clone)]
pub struct ExecutionPlan {
plan: Plan,
- vtables: Option<HashMap<usize, VirtualTable>>,
+ vtables: Option<HashMap<usize, Rc<VirtualTable>>>,
}
impl From<Plan> for ExecutionPlan {
@@ -34,15 +35,15 @@ impl ExecutionPlan {
}
#[must_use]
- pub fn get_vtables(&self) -> Option<&HashMap<usize, VirtualTable>> {
+ pub fn get_vtables(&self) -> Option<&HashMap<usize, Rc<VirtualTable>>> {
self.vtables.as_ref()
}
- pub fn get_mut_vtables(&mut self) -> Option<&mut HashMap<usize, VirtualTable>> {
+ pub fn get_mut_vtables(&mut self) -> Option<&mut HashMap<usize, Rc<VirtualTable>>> {
self.vtables.as_mut()
}
- pub fn set_vtables(&mut self, vtables: HashMap<usize, VirtualTable>) {
+ pub fn set_vtables(&mut self, vtables: HashMap<usize, Rc<VirtualTable>>) {
self.vtables = Some(vtables);
}
@@ -50,10 +51,13 @@ impl ExecutionPlan {
///
/// # Errors
/// - Failed to find a virtual table for the motion node.
- pub fn get_motion_vtable(&self, motion_id: usize) -> Result<&VirtualTable, QueryPlannerError> {
+ pub fn get_motion_vtable(
+ &self,
+ motion_id: usize,
+ ) -> Result<Rc<VirtualTable>, QueryPlannerError> {
if let Some(vtable) = &self.vtables {
if let Some(result) = vtable.get(&motion_id) {
- return Ok(result);
+ return Ok(Rc::clone(result));
}
}
--
GitLab