Skip to content

Commit 50d2c20

Browse files
jpschorrjpschorr
authored
Improve performance by removing extraneous clones and tweaking initial buffer sizes. (#284)
* Remove some extraneous `clone`s. * Increate initial parse node location buffer.
1 parent 4a49f5b commit 50d2c20

File tree

3 files changed

+59
-53
lines changed

3 files changed

+59
-53
lines changed

‎partiql-eval/src/eval.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -473,7 +473,7 @@ impl Evaluable for EvalFilter {
473473

474474
let mut out = partiql_bag![];
475475
for v in input_value.into_iter() {
476-
if self.eval_filter(&v.clone().coerce_to_tuple(), ctx) {
476+
if self.eval_filter(&v.as_tuple_ref(), ctx) {
477477
out.push(v);
478478
}
479479
}
@@ -823,7 +823,7 @@ impl EvalDistinct {
823823

824824
impl Evaluable for EvalDistinct {
825825
fn evaluate(&mut self, _ctx: &dyn EvalContext) -> Option<Value> {
826-
let out = self.input.clone().unwrap();
826+
let out = self.input.take().unwrap();
827827
let u: Vec<Value> = out.into_iter().unique().collect();
828828
Some(Value::Bag(Box::new(Bag::from(u))))
829829
}
@@ -841,7 +841,7 @@ pub struct EvalSink {
841841

842842
impl Evaluable for EvalSink {
843843
fn evaluate(&mut self, _ctx: &dyn EvalContext) -> Option<Value> {
844-
self.input.clone()
844+
self.input.take()
845845
}
846846

847847
fn update_input(&mut self, input: Value, _branch_num: u8) {

‎partiql-eval/src/plan.rs

Lines changed: 55 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@ use partiql_value::Value::Null;
2424
pub struct EvaluatorPlanner;
2525

2626
impl EvaluatorPlanner {
27+
#[inline]
2728
pub fn compile(&self, plan: &LogicalPlan<BindingsOp>) -> EvalPlan {
2829
self.plan_eval(plan)
2930
}
@@ -60,28 +61,28 @@ impl EvaluatorPlanner {
6061
}) => {
6162
if let Some(at_key) = at_key {
6263
Box::new(eval::EvalScan::new_with_at_key(
63-
self.plan_values(expr.clone()),
64+
self.plan_values(expr),
6465
as_key,
6566
at_key,
6667
))
6768
} else {
68-
Box::new(eval::EvalScan::new(self.plan_values(expr.clone()), as_key))
69+
Box::new(eval::EvalScan::new(self.plan_values(expr), as_key))
6970
}
7071
}
7172
BindingsOp::Project(logical::Project { exprs }) => {
7273
let exprs: HashMap<_, _> = exprs
7374
.iter()
74-
.map(|(k, v)| (k.clone(), self.plan_values(v.clone())))
75+
.map(|(k, v)| (k.clone(), self.plan_values(v)))
7576
.collect();
7677
Box::new(eval::EvalSelect::new(exprs))
7778
}
7879
BindingsOp::ProjectAll => Box::new(eval::EvalSelectAll::new()),
7980
BindingsOp::ProjectValue(logical::ProjectValue { expr }) => {
80-
let expr = self.plan_values(expr.clone());
81+
let expr = self.plan_values(expr);
8182
Box::new(eval::EvalSelectValue::new(expr))
8283
}
8384
BindingsOp::Filter(logical::Filter { expr }) => Box::new(eval::EvalFilter {
84-
expr: self.plan_values(expr.clone()),
85+
expr: self.plan_values(expr),
8586
input: None,
8687
}),
8788
BindingsOp::Distinct => Box::new(eval::EvalDistinct::new()),
@@ -91,7 +92,7 @@ impl EvaluatorPlanner {
9192
as_key,
9293
at_key,
9394
}) => Box::new(eval::EvalUnpivot::new(
94-
self.plan_values(expr.clone()),
95+
self.plan_values(expr),
9596
as_key,
9697
at_key.clone(),
9798
)),
@@ -111,7 +112,7 @@ impl EvaluatorPlanner {
111112
};
112113
let on = on
113114
.as_ref()
114-
.map(|on_condition| self.plan_values(on_condition.clone()));
115+
.map(|on_condition| self.plan_values(on_condition));
115116
Box::new(eval::EvalJoin::new(
116117
kind,
117118
self.get_eval_node(left),
@@ -120,7 +121,7 @@ impl EvaluatorPlanner {
120121
))
121122
}
122123
BindingsOp::ExprQuery(logical::ExprQuery { expr }) => {
123-
let expr = self.plan_values(expr.clone());
124+
let expr = self.plan_values(expr);
124125
Box::new(eval::EvalExprQuery::new(expr))
125126
}
126127
BindingsOp::OrderBy => todo!("OrderBy"),
@@ -131,10 +132,10 @@ impl EvaluatorPlanner {
131132
}
132133
}
133134

134-
fn plan_values(&self, ve: ValueExpr) -> Box<dyn EvalExpr> {
135+
fn plan_values(&self, ve: &ValueExpr) -> Box<dyn EvalExpr> {
135136
match ve {
136137
ValueExpr::UnExpr(unary_op, operand) => {
137-
let operand = self.plan_values(*operand);
138+
let operand = self.plan_values(operand);
138139
let op = match unary_op {
139140
UnaryOp::Pos => EvalUnaryOp::Pos,
140141
UnaryOp::Neg => EvalUnaryOp::Neg,
@@ -143,8 +144,8 @@ impl EvaluatorPlanner {
143144
Box::new(EvalUnaryOpExpr { op, operand })
144145
}
145146
ValueExpr::BinaryExpr(binop, lhs, rhs) => {
146-
let lhs = self.plan_values(*lhs);
147-
let rhs = self.plan_values(*rhs);
147+
let lhs = self.plan_values(lhs);
148+
let rhs = self.plan_values(rhs);
148149
let op = match binop {
149150
BinaryOp::And => EvalBinOp::And,
150151
BinaryOp::Or => EvalBinOp::Or,
@@ -165,67 +166,67 @@ impl EvaluatorPlanner {
165166
};
166167
Box::new(EvalBinOpExpr { op, lhs, rhs })
167168
}
168-
ValueExpr::Lit(lit) => Box::new(EvalLitExpr { lit }),
169+
ValueExpr::Lit(lit) => Box::new(EvalLitExpr { lit: lit.clone() }),
169170
ValueExpr::Path(expr, components) => Box::new(EvalPath {
170-
expr: self.plan_values(*expr),
171+
expr: self.plan_values(expr),
171172
components: components
172-
.into_iter()
173+
.iter()
173174
.map(|c| match c {
174-
PathComponent::Key(k) => eval::EvalPathComponent::Key(k),
175-
PathComponent::Index(i) => eval::EvalPathComponent::Index(i),
175+
PathComponent::Key(k) => eval::EvalPathComponent::Key(k.clone()),
176+
PathComponent::Index(i) => eval::EvalPathComponent::Index(*i),
176177
PathComponent::KeyExpr(k) => {
177-
eval::EvalPathComponent::KeyExpr(self.plan_values(*k))
178+
eval::EvalPathComponent::KeyExpr(self.plan_values(k))
178179
}
179180
PathComponent::IndexExpr(i) => {
180-
eval::EvalPathComponent::IndexExpr(self.plan_values(*i))
181+
eval::EvalPathComponent::IndexExpr(self.plan_values(i))
181182
}
182183
})
183184
.collect(),
184185
}),
185-
ValueExpr::VarRef(name) => Box::new(EvalVarRef { name }),
186+
ValueExpr::VarRef(name) => Box::new(EvalVarRef { name: name.clone() }),
186187
ValueExpr::TupleExpr(expr) => {
187188
let attrs: Vec<Box<dyn EvalExpr>> = expr
188189
.attrs
189-
.into_iter()
190+
.iter()
190191
.map(|attr| self.plan_values(attr))
191192
.collect();
192193
let vals: Vec<Box<dyn EvalExpr>> = expr
193194
.values
194-
.into_iter()
195+
.iter()
195196
.map(|attr| self.plan_values(attr))
196197
.collect();
197198
Box::new(EvalTupleExpr { attrs, vals })
198199
}
199200
ValueExpr::ListExpr(expr) => {
200201
let elements: Vec<Box<dyn EvalExpr>> = expr
201202
.elements
202-
.into_iter()
203+
.iter()
203204
.map(|elem| self.plan_values(elem))
204205
.collect();
205206
Box::new(EvalListExpr { elements })
206207
}
207208
ValueExpr::BagExpr(expr) => {
208209
let elements: Vec<Box<dyn EvalExpr>> = expr
209210
.elements
210-
.into_iter()
211+
.iter()
211212
.map(|elem| self.plan_values(elem))
212213
.collect();
213214
Box::new(EvalBagExpr { elements })
214215
}
215216
ValueExpr::BetweenExpr(expr) => {
216-
let value = self.plan_values(*expr.value);
217-
let from = self.plan_values(*expr.from);
218-
let to = self.plan_values(*expr.to);
217+
let value = self.plan_values(expr.value.as_ref());
218+
let from = self.plan_values(expr.from.as_ref());
219+
let to = self.plan_values(expr.to.as_ref());
219220
Box::new(EvalBetweenExpr { value, from, to })
220221
}
221222
ValueExpr::PatternMatchExpr(PatternMatchExpr { value, pattern }) => {
222-
let value = self.plan_values(*value);
223+
let value = self.plan_values(value);
223224
match pattern {
224225
Pattern::LIKE(logical::LikeMatch { pattern, escape }) => {
225226
// TODO statically assert escape length
226227
assert!(escape.chars().count() <= 1);
227228
let escape = escape.chars().next();
228-
let regex = like_to_re_pattern(&pattern, escape);
229+
let regex = like_to_re_pattern(pattern, escape);
229230
Box::new(EvalLikeMatch::new(value, &regex))
230231
}
231232
}
@@ -236,56 +237,61 @@ impl EvaluatorPlanner {
236237
ValueExpr::SimpleCase(e) => {
237238
let cases = e
238239
.cases
239-
.into_iter()
240+
.iter()
240241
.map(|case| {
241242
(
242-
self.plan_values(ValueExpr::BinaryExpr(
243+
self.plan_values(&ValueExpr::BinaryExpr(
243244
BinaryOp::Eq,
244245
e.expr.clone(),
245-
case.0,
246+
case.0.clone(),
246247
)),
247-
self.plan_values(*case.1),
248+
self.plan_values(case.1.as_ref()),
248249
)
249250
})
250251
.collect();
251-
let default = match e.default {
252+
let default = match &e.default {
252253
// If no `ELSE` clause is specified, use implicit `ELSE NULL` (see section 6.9, pg 142 of SQL-92 spec)
253254
None => Box::new(EvalLitExpr {
254255
lit: Box::new(Null),
255256
}),
256-
Some(def) => self.plan_values(*def),
257+
Some(def) => self.plan_values(def),
257258
};
258259
// Here, rewrite `SimpleCaseExpr`s as `SearchedCaseExpr`s
259260
Box::new(EvalSearchedCaseExpr { cases, default })
260261
}
261262
ValueExpr::SearchedCase(e) => {
262263
let cases = e
263264
.cases
264-
.into_iter()
265-
.map(|case| (self.plan_values(*case.0), self.plan_values(*case.1)))
265+
.iter()
266+
.map(|case| {
267+
(
268+
self.plan_values(case.0.as_ref()),
269+
self.plan_values(case.1.as_ref()),
270+
)
271+
})
266272
.collect();
267-
let default = match e.default {
273+
let default = match &e.default {
268274
// If no `ELSE` clause is specified, use implicit `ELSE NULL` (see section 6.9, pg 142 of SQL-92 spec)
269275
None => Box::new(EvalLitExpr {
270276
lit: Box::new(Null),
271277
}),
272-
Some(def) => self.plan_values(*def),
278+
Some(def) => self.plan_values(def.as_ref()),
273279
};
274280
Box::new(EvalSearchedCaseExpr { cases, default })
275281
}
276282
ValueExpr::IsTypeExpr(i) => {
277-
let expr = self.plan_values(*i.expr);
283+
let expr = self.plan_values(i.expr.as_ref());
278284
match i.not {
279285
true => Box::new(EvalUnaryOpExpr {
280286
op: EvalUnaryOp::Not,
281287
operand: Box::new(EvalIsTypeExpr {
282288
expr,
283-
is_type: i.is_type,
289+
is_type: i.is_type.clone(),
284290
}),
285291
}),
286292
false => Box::new(EvalIsTypeExpr {
287293
expr,
288-
is_type: i.is_type,
294+
is_type: i.is_type.clone(),
289295
}),
290296
}
291297
}
@@ -297,14 +303,14 @@ impl EvaluatorPlanner {
297303
cases: vec![(
298304
Box::new(ValueExpr::BinaryExpr(
299305
BinaryOp::Eq,
300-
Box::new(*n.lhs.clone()),
301-
Box::new(*n.rhs.clone()),
306+
n.lhs.clone(),
307+
n.rhs.clone(),
302308
)),
303309
Box::new(ValueExpr::Lit(Box::new(Null))),
304310
)],
305-
default: Some(Box::new(*n.lhs)),
311+
default: Some(n.lhs.clone()),
306312
});
307-
self.plan_values(rewritten_as_case)
313+
self.plan_values(&rewritten_as_case)
308314
}
309315
ValueExpr::CoalesceExpr(c) => {
310316
// COALESCE can be rewritten using CASE WHEN expressions as per section 6.9 pg 142 of SQL-92 spec:
@@ -329,19 +335,19 @@ impl EvaluatorPlanner {
329335
};
330336
ValueExpr::SearchedCase(sc)
331337
}
332-
self.plan_values(as_case(c.elements.first().unwrap(), &c.elements[1..]))
338+
self.plan_values(&as_case(c.elements.first().unwrap(), &c.elements[1..]))
333339
}
334340
ValueExpr::DynamicLookup(lookups) => {
335341
let lookups = lookups
336-
.into_iter()
342+
.iter()
337343
.map(|lookup| self.plan_values(lookup))
338344
.collect_vec();
339345

340346
Box::new(EvalDynamicLookup { lookups })
341347
}
342348
ValueExpr::Call(logical::CallExpr { name, arguments }) => {
343349
let mut args = arguments
344-
.into_iter()
350+
.iter()
345351
.map(|arg| self.plan_values(arg))
346352
.collect_vec();
347353
match name {

‎partiql-parser/src/parse/parser_state.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ type ParseErrorRecovery<'input> =
1515
ErrorRecovery<ByteOffset, lexer::Token<'input>, ParseError<'input, BytePosition>>;
1616
type ParseErrors<'input> = Vec<ParseErrorRecovery<'input>>;
1717

18-
const INIT_LOCATIONS: usize = 50;
18+
const INIT_LOCATIONS: usize = 100;
1919

2020
/// A provider of 'fresh' [`NodeId`]s.
2121
// NOTE `pub` instead of `pub(crate)` only because LALRPop's generated code uses this in `pub trait __ToTriple`

0 commit comments

Comments
 (0)