Skip to content

Commit 5a61189

Browse files
committed
perf(formatter): avoid unnecessary allocation for BinaryLikeExpression (#15467)
Previously, we split the binary expression into a few parts (at least two) and stored them in a `Vec`. In some situations, the `Vec` will be printed directly without using the `Vec` API. This means that storing them in a `Vec` is an unnecessary allocation. This PR eliminates this kind of allocation by introducing `format_flattened_logical_expression`, which won't store any part in a `Vec`.
1 parent 012f7a5 commit 5a61189

File tree

1 file changed

+114
-57
lines changed

1 file changed

+114
-57
lines changed

crates/oxc_formatter/src/write/binary_like_expression.rs

Lines changed: 114 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -214,11 +214,15 @@ impl<'a> Format<'a> for BinaryLikeExpression<'a, '_> {
214214
fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
215215
let parent = self.parent();
216216
let is_inside_condition = self.is_inside_condition(parent);
217-
let parts = split_into_left_and_right_sides(*self, is_inside_condition);
218217

219218
// Don't indent inside of conditions because conditions add their own indent and grouping.
220219
if is_inside_condition {
221-
return write!(f, [&format_once(|f| { f.join().entries(parts).finish() })]);
220+
return write!(
221+
f,
222+
[&format_once(|f| {
223+
format_flattened_logical_expression(*self, is_inside_condition, f)
224+
})]
225+
);
222226
}
223227

224228
// Add a group with a soft block indent in cases where it is necessary to parenthesize the binary expression.
@@ -233,53 +237,74 @@ impl<'a> Format<'a> for BinaryLikeExpression<'a, '_> {
233237
if is_inside_parenthesis {
234238
return write!(
235239
f,
236-
[group(&soft_block_indent(&format_once(|f| { f.join().entries(parts).finish() })))]
240+
[group(&soft_block_indent(&format_once(|f| {
241+
// is_inside_condition is always false here (we returned early if true)
242+
format_flattened_logical_expression(*self, false, f)
243+
})))]
237244
);
238245
}
239246

240-
if self.should_not_indent_if_parent_indents(self.parent()) || {
241-
let flattened = parts.len() > 2;
242-
let inline_logical_expression = self.should_inline_logical_expression();
243-
let should_indent_if_inlines = should_indent_if_parent_inlines(self.parent());
244-
(inline_logical_expression && !flattened)
245-
|| (!inline_logical_expression && should_indent_if_inlines)
246-
} {
247+
// Check if we can take another early-exit path before building the Vec
248+
let should_not_indent = self.should_not_indent_if_parent_indents(self.parent());
249+
250+
// Optimize: if should_not_indent is true, we can skip building the Vec entirely
251+
if should_not_indent {
252+
return write!(
253+
f,
254+
[group(&format_once(|f| {
255+
// is_inside_condition is always false here (we returned early if true)
256+
format_flattened_logical_expression(*self, false, f)
257+
}))]
258+
);
259+
}
260+
261+
// Check other conditions that require knowing if the expression is flattened
262+
let inline_logical_expression = self.should_inline_logical_expression();
263+
let should_indent_if_inlines = should_indent_if_parent_inlines(self.parent());
264+
265+
// We need to know if it's flattened to make this decision, so we must build parts
266+
// is_inside_condition is always false here (we returned early if true)
267+
let parts = split_into_left_and_right_sides(*self, false);
268+
let flattened = parts.len() > 2;
269+
270+
if (inline_logical_expression && !flattened)
271+
|| (!inline_logical_expression && should_indent_if_inlines)
272+
{
247273
return write!(f, [group(&format_once(|f| { f.join().entries(parts).finish() }))]);
248274
}
249275

250-
if let Some(first) = parts.first() {
251-
let last_is_jsx = parts.last().is_some_and(BinaryLeftOrRightSide::is_jsx);
252-
let tail_parts = if last_is_jsx { &parts[1..parts.len() - 1] } else { &parts[1..] };
276+
// `parts` is guaranteed to have at least 2 elements (Left + Right)
277+
// split_into_left_and_right_sides always pushes at least one Right,
278+
// and either pushes Left or recursively adds items that include Left(s)
279+
let first = &parts[0];
280+
let last_is_jsx = parts.last().is_some_and(BinaryLeftOrRightSide::is_jsx);
281+
let tail_parts = if last_is_jsx { &parts[1..parts.len() - 1] } else { &parts[1..] };
253282

254-
let group_id = f.group_id("logicalChain");
283+
let group_id = f.group_id("logicalChain");
255284

256-
let format_non_jsx_parts = format_with(|f| {
257-
write!(
258-
f,
259-
[group(&format_args!(
260-
first,
261-
indent(&format_once(|f| { f.join().entries(tail_parts.iter()).finish() }))
262-
))
263-
.with_group_id(Some(group_id))]
264-
)
265-
});
266-
267-
if last_is_jsx {
268-
// `last_is_jsx` is only true if parts is not empty
269-
let jsx_element = parts.last().unwrap();
270-
write!(
271-
f,
272-
[group(&format_args!(
273-
format_non_jsx_parts,
274-
indent_if_group_breaks(&jsx_element, group_id),
275-
))]
276-
)
277-
} else {
278-
write!(f, format_non_jsx_parts)
279-
}
285+
let format_non_jsx_parts = format_with(|f| {
286+
write!(
287+
f,
288+
[group(&format_args!(
289+
first,
290+
indent(&format_once(|f| { f.join().entries(tail_parts.iter()).finish() }))
291+
))
292+
.with_group_id(Some(group_id))]
293+
)
294+
});
295+
296+
if last_is_jsx {
297+
// `last_is_jsx` is only true if parts is not empty (which is always the case)
298+
let jsx_element = parts.last().unwrap();
299+
write!(
300+
f,
301+
[group(&format_args!(
302+
format_non_jsx_parts,
303+
indent_if_group_breaks(&jsx_element, group_id),
304+
))]
305+
)
280306
} else {
281-
// Empty, should never ever happen but let's gracefully recover.
282-
Ok(())
307+
write!(f, format_non_jsx_parts)
283308
}
284309
}
285310
}
@@ -301,6 +326,36 @@ enum BinaryLeftOrRightSide<'a, 'b> {
301326
},
302327
}
303328

329+
/// Formats a flattened logical expression directly without allocating a Vec.
330+
/// This is used for nested logical expressions with the same operator to avoid
331+
/// the overhead of building a Vec just to immediately iterate over it.
332+
fn format_flattened_logical_expression<'a>(
333+
binary: BinaryLikeExpression<'a, '_>,
334+
inside_condition: bool,
335+
f: &mut Formatter<'_, 'a>,
336+
) -> FormatResult<()> {
337+
fn format_recursive<'a>(
338+
binary: BinaryLikeExpression<'a, '_>,
339+
inside_condition: bool,
340+
f: &mut Formatter<'_, 'a>,
341+
) -> FormatResult<()> {
342+
let left = binary.left();
343+
344+
if binary.can_flatten() {
345+
// Recursively format nested binary expressions
346+
format_recursive(BinaryLikeExpression::try_from(left).unwrap(), inside_condition, f)?;
347+
} else {
348+
// Format the left terminal
349+
write!(f, [group(left)])?;
350+
}
351+
352+
// Format the right side with operator
353+
BinaryLeftOrRightSide::Right { parent: binary, inside_condition }.fmt(f)
354+
}
355+
356+
format_recursive(binary, inside_condition, f)
357+
}
358+
304359
impl<'a> Format<'a> for BinaryLeftOrRightSide<'a, '_> {
305360
fn fmt(&self, f: &mut Formatter<'_, 'a>) -> FormatResult<()> {
306361
match self {
@@ -362,21 +417,22 @@ impl<'a> Format<'a> for BinaryLeftOrRightSide<'a, '_> {
362417
soft_line_break_or_space(),
363418
format_once(|f| {
364419
// If the left side of the right logical expression is still a logical expression with
365-
// the same operator, we need to recursively split it into left and right sides.
420+
// the same operator, we need to recursively format it inline.
366421
// This way, we can ensure that all parts are in the same group.
422+
// We format directly instead of allocating a Vec via split_into_left_and_right_sides.
367423
let left_child = right_logical.left();
368424
if let AstNodes::LogicalExpression(left_logical_child) =
369425
left_child.as_ast_nodes()
370426
&& operator == left_logical_child.operator()
371427
{
372-
let left_parts = split_into_left_and_right_sides(
428+
// Format the nested logical expression inline without Vec allocation
429+
format_flattened_logical_expression(
373430
BinaryLikeExpression::LogicalExpression(
374431
left_logical_child,
375432
),
376433
*inside_parenthesis,
377-
);
378-
379-
f.join().entries(left_parts).finish()
434+
f,
435+
)
380436
} else {
381437
left_child.fmt(f)
382438
}
@@ -410,19 +466,20 @@ impl<'a> Format<'a> for BinaryLeftOrRightSide<'a, '_> {
410466
write!(f, right)
411467
});
412468

469+
// Cache as_ast_nodes() calls to avoid repeated conversions
470+
let left_ast_nodes = binary_like_expression.left().as_ast_nodes();
471+
let right_ast_nodes = right.as_ast_nodes();
472+
413473
// Doesn't match prettier that only distinguishes between logical and binary
414-
let should_group = !(is_same_binary_expression_kind(
415-
binary_like_expression,
416-
binary_like_expression.parent(),
417-
) || is_same_binary_expression_kind(
418-
binary_like_expression,
419-
binary_like_expression.left().as_ast_nodes(),
420-
) || is_same_binary_expression_kind(
421-
binary_like_expression,
422-
right.as_ast_nodes(),
423-
) || (*inside_parenthesis && logical_operator.is_some()));
424-
425-
match binary_like_expression.left().as_ast_nodes() {
474+
let should_group =
475+
!(is_same_binary_expression_kind(
476+
binary_like_expression,
477+
binary_like_expression.parent(),
478+
) || is_same_binary_expression_kind(binary_like_expression, left_ast_nodes)
479+
|| is_same_binary_expression_kind(binary_like_expression, right_ast_nodes)
480+
|| (*inside_parenthesis && logical_operator.is_some()));
481+
482+
match left_ast_nodes {
426483
AstNodes::LogicalExpression(logical) => {
427484
logical.format_trailing_comments(f)?;
428485
}

0 commit comments

Comments
 (0)