Skip to content

Commit 0df6aff

Browse files
committed
perf: batch code fixes
1 parent 03666fd commit 0df6aff

File tree

6 files changed

+440
-56
lines changed

6 files changed

+440
-56
lines changed

crates/biome_rowan/src/ast/batch.rs

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -590,6 +590,37 @@ where
590590
pub fn is_empty(&self) -> bool {
591591
self.changes.is_empty()
592592
}
593+
594+
/// Merge another mutation's changes into this one.
595+
///
596+
/// Both mutations should operate on trees with the same structure for
597+
/// correct results. The changes from the other mutation are added to this
598+
/// mutation's change queue.
599+
///
600+
/// This is useful for batching multiple non-overlapping fixes into a single
601+
/// commit operation, which can significantly improve performance by reducing
602+
/// the number of tree rebuilds needed.
603+
///
604+
/// # Example
605+
///
606+
/// ```ignore
607+
/// let mut batch1 = tree.begin();
608+
/// batch1.replace_node(node_a, new_a);
609+
///
610+
/// let mut batch2 = tree.begin();
611+
/// batch2.replace_node(node_b, new_b);
612+
///
613+
/// // Merge batch2 into batch1
614+
/// batch1.merge(batch2);
615+
///
616+
/// // Commit all changes at once
617+
/// let new_tree = batch1.commit();
618+
/// ```
619+
pub fn merge(&mut self, other: BatchMutation<L>) {
620+
for change in other.changes {
621+
self.changes.push(change);
622+
}
623+
}
593624
}
594625

595626
#[cfg(test)]
@@ -701,4 +732,51 @@ pub mod test {
701732

702733
assert_eq!(expected_debug, format!("{after:#?}"));
703734
}
735+
736+
#[test]
737+
pub fn ok_batch_mutation_merge_two_mutations() {
738+
let (before, _) = tree_two("a", "b");
739+
let (expected, expected_debug) = tree_two("c", "d");
740+
741+
let a = find(&before, "a");
742+
let b = find(&before, "b");
743+
let c = clone_detach(&expected, "c");
744+
let d = clone_detach(&expected, "d");
745+
746+
// Create two separate mutations
747+
let mut batch1 = before.clone().begin();
748+
batch1.replace_node(a, c);
749+
750+
let mut batch2 = before.begin();
751+
batch2.replace_node(b, d);
752+
753+
// Merge batch2 into batch1
754+
batch1.merge(batch2);
755+
let after = batch1.commit();
756+
757+
// Result should be the same as applying both changes together
758+
assert_eq!(expected_debug, format!("{after:#?}"));
759+
}
760+
761+
#[test]
762+
pub fn ok_batch_mutation_merge_empty_mutation() {
763+
let (before, _) = tree_one("a");
764+
let (expected, expected_debug) = tree_one("b");
765+
766+
let a = find(&before, "a");
767+
let b = clone_detach(&expected, "b");
768+
769+
let mut batch1 = before.clone().begin();
770+
batch1.replace_node(a, b);
771+
772+
// Create an empty mutation
773+
let batch2 = before.begin();
774+
assert!(batch2.is_empty());
775+
776+
// Merge empty mutation should not change anything
777+
batch1.merge(batch2);
778+
let after = batch1.commit();
779+
780+
assert_eq!(expected_debug, format!("{after:#?}"));
781+
}
704782
}

crates/biome_service/src/file_handlers/css.rs

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
use super::{
22
AnalyzerVisitorBuilder, CodeActionsParams, EnabledForPath, ExtensionHandler, FixAllParams,
3-
LintParams, LintResults, ParseResult, ProcessFixAll, ProcessLint, SearchCapabilities, search,
3+
LintParams, LintResults, ParseResult, ProcessFixAllBatched, ProcessLint, SearchCapabilities,
4+
search,
45
};
56
use crate::WorkspaceError;
67
use crate::configuration::to_analyzer_rules;
@@ -16,7 +17,9 @@ use crate::workspace::{
1617
CodeAction, DocumentFileSource, FixFileResult, GetSyntaxTreeResult, PullActionsResult,
1718
};
1819
use biome_analyze::options::PreferredQuote;
19-
use biome_analyze::{AnalysisFilter, AnalyzerConfiguration, AnalyzerOptions, ControlFlow, Never};
20+
use biome_analyze::{
21+
AnalysisFilter, AnalyzerConfiguration, AnalyzerOptions, ControlFlow, Never, RuleError,
22+
};
2023
use biome_configuration::css::{
2124
CssAllowWrongLineCommentsEnabled, CssAssistConfiguration, CssAssistEnabled,
2225
CssFormatterConfiguration, CssFormatterEnabled, CssLinterConfiguration, CssLinterEnabled,
@@ -675,6 +678,9 @@ pub(crate) fn code_actions(params: CodeActionsParams) -> PullActionsResult {
675678
}
676679

677680
/// Applies all the safe fixes to the given syntax tree.
681+
///
682+
/// This uses a batched approach that collects multiple non-overlapping fixes
683+
/// per analysis pass for improved performance.
678684
pub(crate) fn fix_all(params: FixAllParams) -> Result<FixFileResult, WorkspaceError> {
679685
let mut tree: CssRoot = params.parse.tree();
680686
let Some(file_source) = params.document_file_source.to_css_file_source() else {
@@ -704,7 +710,7 @@ pub(crate) fn fix_all(params: FixAllParams) -> Result<FixFileResult, WorkspaceEr
704710
range: None,
705711
};
706712

707-
let mut process_fix_all = ProcessFixAll::new(
713+
let mut process = ProcessFixAllBatched::<CssLanguage>::new(
708714
&params,
709715
rules,
710716
tree.syntax().text_range_with_trivia().len().into(),
@@ -719,25 +725,22 @@ pub(crate) fn fix_all(params: FixAllParams) -> Result<FixFileResult, WorkspaceEr
719725
file_source,
720726
};
721727

722-
let (action, _) = analyze(
728+
// Analyze and collect ALL applicable actions (never breaks early)
729+
let (_, _) = analyze(
723730
&tree,
724731
filter,
725732
&analyzer_options,
726733
css_services,
727734
&params.plugins,
728-
|signal| process_fix_all.process_signal(signal),
735+
|signal| process.collect_signal(signal),
729736
);
730737

731-
let result = process_fix_all.process_action(action, |root| {
732-
tree = match CssRoot::cast(root) {
733-
Some(tree) => tree,
734-
None => return None,
735-
};
736-
Some(tree.syntax().text_range_with_trivia().len().into())
737-
})?;
738+
// Get non-overlapping batch of actions
739+
let batch = process.take_batch();
738740

739-
if result.is_none() {
740-
return process_fix_all.finish(|| {
741+
if batch.is_empty() {
742+
// No more fixes to apply - we're done
743+
return process.finish(|| {
741744
Ok(if params.should_format {
742745
Either::Left(format_node(
743746
params.settings.format_options::<CssLanguage>(
@@ -751,6 +754,23 @@ pub(crate) fn fix_all(params: FixAllParams) -> Result<FixFileResult, WorkspaceEr
751754
})
752755
});
753756
}
757+
758+
// Merge all actions into one mutation and apply
759+
if let Some(combined_mutation) = process.merge_actions(batch) {
760+
let (new_root, _) = combined_mutation.commit_with_text_range_and_edit(true);
761+
tree = match CssRoot::cast(new_root) {
762+
Some(tree) => tree,
763+
None => {
764+
return Err(WorkspaceError::RuleError(
765+
RuleError::ReplacedRootWithNonRootError { rule_name: None },
766+
));
767+
}
768+
};
769+
770+
// Check for runaway growth
771+
let new_len: u32 = tree.syntax().text_range_with_trivia().len().into();
772+
process.check_growth(new_len)?;
773+
}
754774
}
755775
}
756776

crates/biome_service/src/file_handlers/graphql.rs

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use super::{
22
AnalyzerVisitorBuilder, CodeActionsParams, DocumentFileSource, EnabledForPath,
3-
ExtensionHandler, FixAllParams, LintParams, LintResults, ParseResult, ProcessFixAll,
3+
ExtensionHandler, FixAllParams, LintParams, LintResults, ParseResult, ProcessFixAllBatched,
44
ProcessLint, SearchCapabilities,
55
};
66
use crate::WorkspaceError;
@@ -14,7 +14,9 @@ use crate::settings::{
1414
Settings, check_feature_activity, check_override_feature_activity,
1515
};
1616
use crate::workspace::{CodeAction, FixFileResult, GetSyntaxTreeResult, PullActionsResult};
17-
use biome_analyze::{AnalysisFilter, AnalyzerConfiguration, AnalyzerOptions, ControlFlow, Never};
17+
use biome_analyze::{
18+
AnalysisFilter, AnalyzerConfiguration, AnalyzerOptions, ControlFlow, Never, RuleError,
19+
};
1820
use biome_configuration::graphql::{
1921
GraphqlAssistConfiguration, GraphqlAssistEnabled, GraphqlFormatterConfiguration,
2022
GraphqlFormatterEnabled, GraphqlLinterConfiguration, GraphqlLinterEnabled,
@@ -541,6 +543,9 @@ pub(crate) fn code_actions(params: CodeActionsParams) -> PullActionsResult {
541543
}
542544

543545
/// If applies all the safe fixes to the given syntax tree.
546+
///
547+
/// This uses a batched approach that collects multiple non-overlapping fixes
548+
/// per analysis pass for improved performance.
544549
pub(crate) fn fix_all(params: FixAllParams) -> Result<FixFileResult, WorkspaceError> {
545550
let mut tree: GraphqlRoot = params.parse.tree();
546551

@@ -567,26 +572,24 @@ pub(crate) fn fix_all(params: FixAllParams) -> Result<FixFileResult, WorkspaceEr
567572
range: None,
568573
};
569574

570-
let mut process_fix_all = ProcessFixAll::new(
575+
let mut process = ProcessFixAllBatched::<GraphqlLanguage>::new(
571576
&params,
572577
rules,
573578
tree.syntax().text_range_with_trivia().len().into(),
574579
);
580+
575581
loop {
576-
let (action, _) = analyze(&tree, filter, &analyzer_options, |signal| {
577-
process_fix_all.process_signal(signal)
582+
// Analyze and collect ALL applicable actions (never breaks early)
583+
let (_, _) = analyze(&tree, filter, &analyzer_options, |signal| {
584+
process.collect_signal(signal)
578585
});
579586

580-
let result = process_fix_all.process_action(action, |root| {
581-
tree = match GraphqlRoot::cast(root) {
582-
Some(tree) => tree,
583-
None => return None,
584-
};
585-
Some(tree.syntax().text_range_with_trivia().len().into())
586-
})?;
587+
// Get non-overlapping batch of actions
588+
let batch = process.take_batch();
587589

588-
if result.is_none() {
589-
return process_fix_all.finish(|| {
590+
if batch.is_empty() {
591+
// No more fixes to apply - we're done
592+
return process.finish(|| {
590593
Ok(if params.should_format {
591594
Either::Left(format_node(
592595
params.settings.format_options::<GraphqlLanguage>(
@@ -600,5 +603,22 @@ pub(crate) fn fix_all(params: FixAllParams) -> Result<FixFileResult, WorkspaceEr
600603
})
601604
});
602605
}
606+
607+
// Merge all actions into one mutation and apply
608+
if let Some(combined_mutation) = process.merge_actions(batch) {
609+
let (new_root, _) = combined_mutation.commit_with_text_range_and_edit(true);
610+
tree = match GraphqlRoot::cast(new_root) {
611+
Some(tree) => tree,
612+
None => {
613+
return Err(WorkspaceError::RuleError(
614+
RuleError::ReplacedRootWithNonRootError { rule_name: None },
615+
));
616+
}
617+
};
618+
619+
// Check for runaway growth
620+
let new_len: u32 = tree.syntax().text_range_with_trivia().len().into();
621+
process.check_growth(new_len)?;
622+
}
603623
}
604624
}

crates/biome_service/src/file_handlers/html.rs

Lines changed: 34 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ use super::{
22
AnalyzerCapabilities, AnalyzerVisitorBuilder, Capabilities, CodeActionsParams,
33
DebugCapabilities, DocumentFileSource, EnabledForPath, ExtensionHandler, FixAllParams,
44
FormatEmbedNode, FormatterCapabilities, LintParams, LintResults, ParseEmbedResult, ParseResult,
5-
ParserCapabilities, ProcessFixAll, ProcessLint, SearchCapabilities, UpdateSnippetsNodes,
5+
ParserCapabilities, ProcessFixAllBatched, ProcessLint, SearchCapabilities, UpdateSnippetsNodes,
66
};
77
use crate::configuration::to_analyzer_rules;
88
use crate::settings::{OverrideSettings, check_feature_activity, check_override_feature_activity};
@@ -13,7 +13,9 @@ use crate::{
1313
settings::{ServiceLanguage, Settings},
1414
workspace::GetSyntaxTreeResult,
1515
};
16-
use biome_analyze::{AnalysisFilter, AnalyzerConfiguration, AnalyzerOptions, ControlFlow, Never};
16+
use biome_analyze::{
17+
AnalysisFilter, AnalyzerConfiguration, AnalyzerOptions, ControlFlow, Never, RuleError,
18+
};
1719
use biome_configuration::html::{
1820
HtmlAssistConfiguration, HtmlAssistEnabled, HtmlFormatterConfiguration, HtmlFormatterEnabled,
1921
HtmlLinterConfiguration, HtmlLinterEnabled, HtmlParseInterpolation, HtmlParserConfiguration,
@@ -853,6 +855,10 @@ pub(crate) fn code_actions(params: CodeActionsParams) -> PullActionsResult {
853855
PullActionsResult { actions }
854856
}
855857

858+
/// If applies all the safe fixes to the given syntax tree.
859+
///
860+
/// This uses a batched approach that collects multiple non-overlapping fixes
861+
/// per analysis pass for improved performance.
856862
#[tracing::instrument(level = "debug", skip(params))]
857863
pub(crate) fn fix_all(params: FixAllParams) -> Result<FixFileResult, WorkspaceError> {
858864
let mut tree: HtmlRoot = params.parse.tree();
@@ -880,27 +886,24 @@ pub(crate) fn fix_all(params: FixAllParams) -> Result<FixFileResult, WorkspaceEr
880886
range: None,
881887
};
882888

883-
let mut process_fix_all = ProcessFixAll::new(
889+
let mut process = ProcessFixAllBatched::<HtmlLanguage>::new(
884890
&params,
885891
rules,
886892
tree.syntax().text_range_with_trivia().len().into(),
887893
);
888894

889895
loop {
890-
let (action, _) = analyze(&tree, filter, &analyzer_options, |signal| {
891-
process_fix_all.process_signal(signal)
896+
// Analyze and collect ALL applicable actions (never breaks early)
897+
let (_, _) = analyze(&tree, filter, &analyzer_options, |signal| {
898+
process.collect_signal(signal)
892899
});
893900

894-
let result = process_fix_all.process_action(action, |root| {
895-
tree = match HtmlRoot::cast(root) {
896-
Some(tree) => tree,
897-
None => return None,
898-
};
899-
Some(tree.syntax().text_range_with_trivia().len().into())
900-
})?;
901+
// Get non-overlapping batch of actions
902+
let batch = process.take_batch();
901903

902-
if result.is_none() {
903-
return process_fix_all.finish(|| {
904+
if batch.is_empty() {
905+
// No more fixes to apply - we're done
906+
return process.finish(|| {
904907
Ok(if params.should_format {
905908
Either::Left(format_node(
906909
params.settings.format_options::<HtmlLanguage>(
@@ -915,6 +918,23 @@ pub(crate) fn fix_all(params: FixAllParams) -> Result<FixFileResult, WorkspaceEr
915918
})
916919
});
917920
}
921+
922+
// Merge all actions into one mutation and apply
923+
if let Some(combined_mutation) = process.merge_actions(batch) {
924+
let (new_root, _) = combined_mutation.commit_with_text_range_and_edit(true);
925+
tree = match HtmlRoot::cast(new_root) {
926+
Some(tree) => tree,
927+
None => {
928+
return Err(WorkspaceError::RuleError(
929+
RuleError::ReplacedRootWithNonRootError { rule_name: None },
930+
));
931+
}
932+
};
933+
934+
// Check for runaway growth
935+
let new_len: u32 = tree.syntax().text_range_with_trivia().len().into();
936+
process.check_growth(new_len)?;
937+
}
918938
}
919939
}
920940

0 commit comments

Comments
 (0)