Skip to content

Commit 064f835

Browse files
committed
perf(formatter): optimize printing call arguments (#15464)
`most_flat` variant is only used when the grouped argument isn't broken, so move it to its branch to avoid executing it
1 parent a20fb37 commit 064f835

File tree

1 file changed

+65
-65
lines changed

1 file changed

+65
-65
lines changed

crates/oxc_formatter/src/write/call_arguments.rs

Lines changed: 65 additions & 65 deletions
Original file line numberDiff line numberDiff line change
@@ -689,17 +689,12 @@ fn write_grouped_arguments<'a>(
689689
return format_all_elements_broken_out(node, elements.into_iter(), true, f);
690690
}
691691

692-
// We now cache the delimiter tokens. This is needed because `[crate::best_fitting]` will try to
693-
// print each version first
694-
let l_paren_token = "(".memoized();
695-
let r_paren_token = ")".memoized();
696-
697692
// First write the most expanded variant because it needs `arguments`.
698693
let most_expanded = {
699694
let mut buffer = VecBuffer::new(f.state_mut());
700695
buffer.write_element(FormatElement::Tag(Tag::StartEntry))?;
701696

702-
format_all_elements_broken_out(node, elements.iter().cloned(), true, &mut buffer);
697+
format_all_elements_broken_out(node, elements.iter().cloned(), true, &mut buffer)?;
703698

704699
buffer.write_element(FormatElement::Tag(Tag::EndEntry))?;
705700

@@ -718,71 +713,44 @@ fn write_grouped_arguments<'a>(
718713
match group_layout {
719714
GroupedCallArgumentLayout::GroupedFirstArgument => {
720715
let argument = node.first().unwrap();
721-
let mut first = grouped.first_mut().unwrap();
722-
first.0 = f.intern(&format_once(|f| {
716+
let interned = f.intern(&format_once(|f| {
723717
FormatGroupedFirstArgument { argument }.fmt(f)?;
724718
write!(f, (last_index != 0).then_some(","))
725719
}));
720+
721+
// Turns out, using the grouped layout isn't a good fit because some parameters of the
722+
// grouped function or arrow expression break. In that case, fall back to the all args expanded
723+
// formatting.
724+
// This back tracking is required because testing if the grouped argument breaks would also return `true`
725+
// if any content of the function body breaks. But, as far as this is concerned, it's only interested if
726+
// any content in the signature breaks.
727+
if matches!(interned, Err(FormatError::PoorLayout)) {
728+
return format_all_elements_broken_out(node, grouped.into_iter(), true, f);
729+
}
730+
731+
grouped.first_mut().unwrap().0 = interned;
726732
}
727733
GroupedCallArgumentLayout::GroupedLastArgument => {
728734
let argument = node.last().unwrap();
729-
let mut last = grouped.last_mut().unwrap();
730-
last.0 = f.intern(&format_once(|f| {
735+
let interned = f.intern(&format_once(|f| {
731736
FormatGroupedLastArgument { argument, is_only: only_one_argument }.fmt(f)
732737
}));
733-
}
734-
}
735-
}
736738

737-
// Write the most flat variant with the first or last argument grouped.
738-
let most_flat = {
739-
// let snapshot = f.state_snapshot();
740-
let mut buffer = VecBuffer::new(f.state_mut());
741-
buffer.write_element(FormatElement::Tag(Tag::StartEntry))?;
739+
// Turns out, using the grouped layout isn't a good fit because some parameters of the
740+
// grouped function or arrow expression break. In that case, fall back to the all args expanded
741+
// formatting.
742+
// This back tracking is required because testing if the grouped argument breaks would also return `true`
743+
// if any content of the function body breaks. But, as far as this is concerned, it's only interested if
744+
// any content in the signature breaks.
745+
if matches!(interned, Err(FormatError::PoorLayout)) {
746+
return format_all_elements_broken_out(node, grouped.into_iter(), true, f);
747+
}
742748

743-
let result = write!(
744-
buffer,
745-
[
746-
l_paren_token,
747-
format_with(|f| {
748-
f.join_with(soft_line_break_or_space())
749-
.entries(grouped.iter().map(|(element, _)| {
750-
format_once(|f| {
751-
if let Some(element) = element.clone()? {
752-
f.write_element(element)
753-
} else {
754-
Ok(())
755-
}
756-
})
757-
}))
758-
.finish()
759-
}),
760-
r_paren_token,
761-
]
762-
);
763-
764-
// Turns out, using the grouped layout isn't a good fit because some parameters of the
765-
// grouped function or arrow expression break. In that case, fall back to the all args expanded
766-
// formatting.
767-
// This back tracking is required because testing if the grouped argument breaks would also return `true`
768-
// if any content of the function body breaks. But, as far as this is concerned, it's only interested if
769-
// any content in the signature breaks.
770-
if matches!(result, Err(FormatError::PoorLayout)) {
771-
drop(buffer);
772-
// f.restore_state_snapshot(snapshot);
773-
774-
let mut most_expanded_iter = most_expanded.into_iter();
775-
// Skip over the Start/EndEntry items.
776-
most_expanded_iter.next_back();
777-
778-
// `skip(1)` is skipping the `StartEntry` tag
779-
return f.write_elements(most_expanded_iter.skip(1));
749+
let mut last = grouped.last_mut().unwrap();
750+
grouped.last_mut().unwrap().0 = interned;
751+
}
780752
}
781-
782-
buffer.write_element(FormatElement::Tag(Tag::EndEntry))?;
783-
784-
buffer.into_vec().into_boxed_slice()
785-
};
753+
}
786754

787755
// Write the second variant that forces the group of the first/last argument to expand.
788756
let middle_variant = {
@@ -793,17 +761,17 @@ fn write_grouped_arguments<'a>(
793761
write!(
794762
buffer,
795763
[
796-
l_paren_token,
764+
"(",
797765
format_once(|f| {
798766
let mut joiner = f.join_with(soft_line_break_or_space());
799767

800-
for (i, (element, _)) in grouped.into_iter().enumerate() {
768+
for (i, (element, _)) in grouped.iter().enumerate() {
801769
if (group_layout.is_grouped_first() && i == 0)
802770
|| (group_layout.is_grouped_last() && i == last_index)
803771
{
804772
joiner.entry(
805773
&group(&format_once(|f| {
806-
if let Some(arg_element) = element? {
774+
if let Some(arg_element) = element.clone()? {
807775
f.write_element(arg_element)
808776
} else {
809777
Ok(())
@@ -813,7 +781,7 @@ fn write_grouped_arguments<'a>(
813781
);
814782
} else {
815783
joiner.entry(&format_once(|f| {
816-
if let Some(arg_element) = element? {
784+
if let Some(arg_element) = element.clone()? {
817785
f.write_element(arg_element)
818786
} else {
819787
Ok(())
@@ -824,7 +792,7 @@ fn write_grouped_arguments<'a>(
824792

825793
joiner.finish()
826794
}),
827-
r_paren_token
795+
")"
828796
]
829797
)?;
830798

@@ -839,6 +807,38 @@ fn write_grouped_arguments<'a>(
839807
write!(f, [expand_parent()])?;
840808
vec![middle_variant, most_expanded.into_boxed_slice()]
841809
} else {
810+
// Write the most flat variant with the first or last argument grouped.
811+
let most_flat = {
812+
// let snapshot = f.state_snapshot();
813+
let mut buffer = VecBuffer::new(f.state_mut());
814+
buffer.write_element(FormatElement::Tag(Tag::StartEntry))?;
815+
816+
let result = write!(
817+
buffer,
818+
[
819+
"(",
820+
format_once(|f| {
821+
f.join_with(soft_line_break_or_space())
822+
.entries(grouped.into_iter().map(|(element, _)| {
823+
format_once(move |f| {
824+
if let Some(element) = element? {
825+
f.write_element(element)
826+
} else {
827+
Ok(())
828+
}
829+
})
830+
}))
831+
.finish()
832+
}),
833+
")",
834+
]
835+
);
836+
837+
buffer.write_element(FormatElement::Tag(Tag::EndEntry))?;
838+
839+
buffer.into_vec().into_boxed_slice()
840+
};
841+
842842
vec![most_flat, middle_variant, most_expanded.into_boxed_slice()]
843843
};
844844

0 commit comments

Comments
 (0)