Skip to content

Commit 4cf81b3

Browse files
authored
KCL: Rm clone in parser for 3% speedup (#7857)
Previously, building a member expression required cloning the Expr that became the member expression's object. Now I changed things a bit, so that the object expr doesn't need to be cloned. This is used every time an Expr is parsed, so it's somewhat hot code, and fixing the clone gave a consistent 2-4% speedup across tests on my Macbook Pro.
1 parent da1d933 commit 4cf81b3

File tree

1 file changed

+13
-11
lines changed

1 file changed

+13
-11
lines changed

rust/kcl-lib/src/parsing/parser.rs

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1369,15 +1369,17 @@ fn member_expression_subscript(i: &mut TokenSlice) -> ModalResult<(Expr, usize,
13691369
Ok((property, end, computed))
13701370
}
13711371

1372-
/// Get a property of an object, or an index of an array, or a member of a collection.
1373-
/// Can be arbitrarily nested, e.g. `people[i]['adam'].age`.
1374-
fn build_member_expression(object: Expr, i: &mut TokenSlice) -> ModalResult<Node<MemberExpression>> {
1372+
fn find_members(i: &mut TokenSlice) -> ModalResult<Vec<(Expr, usize, bool)>> {
13751373
// Now a sequence of members.
13761374
let member = alt((member_expression_dot, member_expression_subscript)).context(expected("a member/property, e.g. size.x and size['height'] and size[0] are all different ways to access a member/property of 'size'"));
1377-
let mut members: Vec<_> = repeat(1.., member)
1375+
repeat(1.., member)
13781376
.context(expected("a sequence of at least one members/properties"))
1379-
.parse_next(i)?;
1377+
.parse_next(i)
1378+
}
13801379

1380+
/// Get a property of an object, or an index of an array, or a member of a collection.
1381+
/// Can be arbitrarily nested, e.g. `people[i]['adam'].age`.
1382+
fn build_member_expression(object: Expr, mut members: Vec<(Expr, usize, bool)>) -> Node<MemberExpression> {
13811383
// Process the first member.
13821384
// It's safe to call remove(0), because the vec is created from repeat(1..),
13831385
// which is guaranteed to have >=1 elements.
@@ -1397,7 +1399,7 @@ fn build_member_expression(object: Expr, i: &mut TokenSlice) -> ModalResult<Node
13971399
);
13981400

13991401
// Each remaining member wraps the current member expression inside another member expression.
1400-
Ok(members
1402+
members
14011403
.into_iter()
14021404
// Take the accumulated member expression from the previous iteration,
14031405
// and use it as the `object` of a new, bigger member expression.
@@ -1413,7 +1415,7 @@ fn build_member_expression(object: Expr, i: &mut TokenSlice) -> ModalResult<Node
14131415
end,
14141416
module_id,
14151417
)
1416-
}))
1418+
})
14171419
}
14181420

14191421
/// Find a noncode node which occurs just after a body item,
@@ -2146,8 +2148,8 @@ fn expr_allowed_in_pipe_expr(i: &mut TokenSlice) -> ModalResult<Expr> {
21462148
.context(expected("a KCL expression (but not a pipe expression)"))
21472149
.parse_next(i)?;
21482150

2149-
let maybe_member = build_member_expression(parsed_expr.clone(), i); // TODO: Eliminate this clone.
2150-
if let Ok(mem) = maybe_member {
2151+
if let Ok(Some(members)) = opt(find_members).parse_next(i) {
2152+
let mem = build_member_expression(parsed_expr, members);
21512153
return Ok(Expr::MemberExpression(Box::new(mem)));
21522154
}
21532155
Ok(parsed_expr)
@@ -2170,8 +2172,8 @@ fn possible_operands(i: &mut TokenSlice) -> ModalResult<Expr> {
21702172
"a KCL value which can be used as an argument/operand to an operator",
21712173
))
21722174
.parse_next(i)?;
2173-
let maybe_member = build_member_expression(expr.clone(), i);
2174-
if let Ok(mem) = maybe_member {
2175+
if let Ok(Some(members)) = opt(find_members).parse_next(i) {
2176+
let mem = build_member_expression(expr, members);
21752177
expr = Expr::MemberExpression(Box::new(mem));
21762178
}
21772179

0 commit comments

Comments
 (0)