Skip to content

Commit a6ff113

Browse files
committed
feat(formatter): optimize MemberChain::from_call_expression (#12591)
Optimize the Biome's implementation to reduce some avoidable allocations
1 parent cf78793 commit a6ff113

File tree

1 file changed

+48
-53
lines changed
  • crates/oxc_formatter/src/utils/member_chain

1 file changed

+48
-53
lines changed

crates/oxc_formatter/src/utils/member_chain/mod.rs

Lines changed: 48 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ pub mod chain_member;
22
pub mod groups;
33
pub mod simple_argument;
44

5+
use std::iter;
6+
57
use crate::{
68
JsLabels, best_fitting,
79
formatter::{Buffer, Format, FormatResult, Formatter, prelude::*},
@@ -33,16 +35,17 @@ impl<'a, 'b> MemberChain<'a, 'b> {
3335
f: &Formatter<'_, 'a>,
3436
) -> Self {
3537
let parent = &call_expression.parent;
36-
let mut chain_members = ChainMembersIterator::new(call_expression).collect::<Vec<_>>();
38+
let mut chain_members = chain_members_iter(call_expression).collect::<Vec<_>>();
3739
chain_members.reverse();
3840

3941
// as explained before, the first group is particular, so we calculate it
40-
let (head_group, remaining_members) =
41-
split_members_into_head_and_remaining_groups(chain_members);
42+
let remaining_members_start_index = get_split_index_of_head_and_tail_groups(&chain_members);
4243

4344
// `flattened_items` now contains only the nodes that should have a sequence of
4445
// `[ StaticMemberExpression -> AnyNode + CallExpression ]`
45-
let tail_groups = compute_remaining_groups(remaining_members);
46+
let tail_groups =
47+
compute_remaining_groups(chain_members.drain(remaining_members_start_index..));
48+
let head_group = MemberChainGroup::from(chain_members);
4649

4750
let mut member_chain = Self { head: head_group, tail: tail_groups, root: call_expression };
4851

@@ -241,12 +244,8 @@ impl<'a> Format<'a> for MemberChain<'a, '_> {
241244
}
242245
}
243246

244-
/// Splits the members into two groups:
245-
/// * The head group that contains all nodes that are not a sequence of: `[ StaticMemberExpression -> AnyNode + CallExpression ]`
246-
/// * The remaining members
247-
fn split_members_into_head_and_remaining_groups<'a, 'b>(
248-
mut members: Vec<ChainMember<'a, 'b>>,
249-
) -> (MemberChainGroup<'a, 'b>, Vec<ChainMember<'a, 'b>>) {
247+
/// Returns the index where the head group ends and the tail groups begin.
248+
fn get_split_index_of_head_and_tail_groups(members: &[ChainMember<'_, '_>]) -> usize {
250249
// This where we apply the first two points explained in the description of the main public function.
251250
// We want to keep iterating over the items until we have call expressions
252251
// - `something()()()()`
@@ -269,7 +268,7 @@ fn split_members_into_head_and_remaining_groups<'a, 'b>(
269268
index + 1
270269
});
271270

272-
let first_group_end_index = if members.first().is_some_and(ChainMember::is_call_expression) {
271+
if members.first().is_some_and(ChainMember::is_call_expression) {
273272
non_call_or_array_member_access_start
274273
} else {
275274
// Take as many member access chains as possible
@@ -289,25 +288,24 @@ fn split_members_into_head_and_remaining_groups<'a, 'b>(
289288
});
290289

291290
non_call_or_array_member_access_start + member_end
292-
};
293-
294-
let remaining = members.split_off(first_group_end_index);
295-
(MemberChainGroup::from(members), remaining)
291+
}
296292
}
297293

298294
/// computes groups coming after the first group
299-
fn compute_remaining_groups<'a, 'b>(members: Vec<ChainMember<'a, 'b>>) -> TailChainGroups<'a, 'b> {
295+
fn compute_remaining_groups<'a, 'b>(
296+
members: impl IntoIterator<Item = ChainMember<'a, 'b>>,
297+
) -> TailChainGroups<'a, 'b> {
300298
let mut has_seen_call_expression = false;
301299
let mut groups_builder = MemberChainGroupsBuilder::default();
302300

303301
for member in members {
304302
match member {
305303
// [0] should be appended at the end of the group instead of the
306304
// beginning of the next one
307-
ChainMember::ComputedMember { .. } if is_computed_array_member_access(&member) => {
305+
ChainMember::ComputedMember(_) if is_computed_array_member_access(&member) => {
308306
groups_builder.start_or_continue_group(member);
309307
}
310-
ChainMember::StaticMember { .. } | ChainMember::ComputedMember { .. } => {
308+
ChainMember::StaticMember(_) | ChainMember::ComputedMember(_) => {
311309
// if we have seen a CallExpression, we want to close the group.
312310
// The resultant group will be something like: [ . , then, () ];
313311
// `.` and `then` belong to the previous StaticMemberExpression,
@@ -324,7 +322,7 @@ fn compute_remaining_groups<'a, 'b>(members: Vec<ChainMember<'a, 'b>>) -> TailCh
324322
groups_builder.start_or_continue_group(member);
325323
has_seen_call_expression = true;
326324
}
327-
ChainMember::TSNonNullExpression { .. } => {
325+
ChainMember::TSNonNullExpression(_) => {
328326
groups_builder.start_or_continue_group(member);
329327
}
330328
ChainMember::Node(_) => unreachable!("Remaining members never have a `Node` variant"),
@@ -379,23 +377,15 @@ fn has_short_name(name: &Atom, tab_width: u8) -> bool {
379377
name.as_str().len() <= tab_width as usize
380378
}
381379

382-
struct ChainMembersIterator<'a, 'b> {
383-
root: Option<&'b AstNode<'a, CallExpression<'a>>>,
384-
next: Option<&'b AstNode<'a, Expression<'a>>>,
385-
}
386-
387-
impl<'a, 'b> ChainMembersIterator<'a, 'b> {
388-
fn new(root: &'b AstNode<'a, CallExpression<'a>>) -> Self {
389-
Self { root: Some(root), next: None }
390-
}
391-
}
392-
393-
impl<'a, 'b> Iterator for ChainMembersIterator<'a, 'b> {
394-
type Item = ChainMember<'a, 'b>;
380+
fn chain_members_iter<'a, 'b>(
381+
root: &'b AstNode<'a, CallExpression<'a>>,
382+
) -> impl Iterator<Item = ChainMember<'a, 'b>> {
383+
let mut is_root = true;
384+
let mut next: Option<&'b AstNode<'a, Expression<'a>>> = None;
395385

396-
fn next(&mut self) -> Option<Self::Item> {
397-
let mut handle_call_expression =
398-
|root: bool,
386+
iter::from_fn(move || {
387+
let handle_call_expression =
388+
|position: CallExpressionPosition,
399389
expr: &'b AstNode<'a, CallExpression<'a>>,
400390
next: &mut Option<&'b AstNode<'a, Expression<'a>>>| {
401391
let callee = expr.callee();
@@ -411,42 +401,47 @@ impl<'a, 'b> Iterator for ChainMembersIterator<'a, 'b> {
411401
*next = Some(callee);
412402
}
413403

414-
let position = if root {
415-
CallExpressionPosition::End
416-
} else if !is_chain {
417-
CallExpressionPosition::Start
418-
} else {
419-
CallExpressionPosition::Middle
420-
};
421-
422404
ChainMember::CallExpression { expression: expr, position }
423405
};
424406

425-
if let Some(root) = self.root.take() {
426-
return Some(handle_call_expression(true, root, &mut self.next));
407+
if is_root {
408+
is_root = false;
409+
return Some(handle_call_expression(CallExpressionPosition::End, root, &mut next));
427410
}
428411

429-
let expression = self.next.take()?;
412+
let expression = next.take()?;
430413

431414
let member = match expression.as_ast_nodes() {
432-
AstNodes::CallExpression(expr) => handle_call_expression(false, expr, &mut self.next),
415+
AstNodes::CallExpression(expr) => {
416+
let callee = expr.callee();
417+
let is_chain = matches!(
418+
callee.as_ref(),
419+
Expression::StaticMemberExpression(_)
420+
| Expression::ComputedMemberExpression(_)
421+
| Expression::CallExpression(_)
422+
);
423+
let position = if is_chain {
424+
CallExpressionPosition::Middle
425+
} else {
426+
CallExpressionPosition::Start
427+
};
428+
handle_call_expression(position, expr, &mut next)
429+
}
433430
AstNodes::StaticMemberExpression(expr) => {
434-
self.next = Some(expr.object());
431+
next = Some(expr.object());
435432
ChainMember::StaticMember(expr)
436433
}
437434
AstNodes::ComputedMemberExpression(expr) => {
438-
self.next = Some(expr.object());
435+
next = Some(expr.object());
439436
ChainMember::ComputedMember(expr)
440437
}
441438
AstNodes::TSNonNullExpression(expr) => {
442-
self.next = Some(expr.expression());
439+
next = Some(expr.expression());
443440
ChainMember::TSNonNullExpression(expr)
444441
}
445442
_ => ChainMember::Node(expression),
446443
};
447444

448445
Some(member)
449-
}
446+
})
450447
}
451-
452-
impl std::iter::FusedIterator for ChainMembersIterator<'_, '_> {}

0 commit comments

Comments
 (0)