Skip to content

Commit d8a4ac7

Browse files
authored
KCL: Mostly reuse one string buffer in recaster (#7869)
Before this PR, most of the recast functions looked like this ``` fn recast(&self, options, indentation_level) -> String ``` In other words, when they ran, they allocated and returned a string. This mean every function would perform at least 1 allocation. Now they generally look like this: ``` fn recast(&self, buf: &mut String, options, indentation_level) ``` In other words, they usually accept a string from the caller and append to it, rather than allocating a new one. This reduces allocations (because generally the same single string can be shared across recasts) which saves time (because a huge portion of recast time was just allocating strings, appending their contents to some other string, then dropping it). On my Macbook, this cuts down the recast benchmarks by 55-65%.
1 parent 238c9f2 commit d8a4ac7

File tree

7 files changed

+597
-444
lines changed

7 files changed

+597
-444
lines changed

rust/kcl-lib/src/lib.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -256,11 +256,11 @@ impl Program {
256256

257257
pub fn recast(&self) -> String {
258258
// Use the default options until we integrate into the UI the ability to change them.
259-
self.ast.recast(&Default::default(), 0)
259+
self.ast.recast_top(&Default::default(), 0)
260260
}
261261

262262
pub fn recast_with_options(&self, options: &FormatOptions) -> String {
263-
self.ast.recast(options, 0)
263+
self.ast.recast_top(options, 0)
264264
}
265265

266266
/// Create an empty program.

rust/kcl-lib/src/lint/checks/camel_case.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ fn lint_lower_camel_case_var(decl: &VariableDeclarator, prog: &AstNode<Program>)
3535

3636
let mut prog = prog.clone();
3737
prog.rename_symbol(&new_name, ident.start);
38-
let recast = prog.recast(&Default::default(), 0);
38+
let recast = prog.recast_top(&Default::default(), 0);
3939

4040
let suggestion = Suggestion {
4141
title: format!("rename '{name}' to '{new_name}'"),

rust/kcl-lib/src/lsp/kcl/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -860,7 +860,7 @@ impl Backend {
860860
// Now let's perform the rename on the ast.
861861
ast.rename_symbol(new_name, pos);
862862
// Now recast it.
863-
let recast = ast.recast(&Default::default(), 0);
863+
let recast = ast.recast_top(&Default::default(), 0);
864864

865865
Ok(Some((current_code.to_string(), recast)))
866866
}
@@ -1480,7 +1480,7 @@ impl LanguageServer for Backend {
14801480
return Ok(None);
14811481
};
14821482
// Now recast it.
1483-
let recast = ast.recast(
1483+
let recast = ast.recast_top(
14841484
&crate::parsing::ast::types::FormatOptions {
14851485
tab_size: params.options.tab_size as usize,
14861486
insert_final_newline: params.options.insert_final_newline.unwrap_or(false),

rust/kcl-lib/src/parsing/ast/types/mod.rs

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -988,7 +988,13 @@ pub enum Expr {
988988

989989
impl Expr {
990990
pub fn get_lsp_folding_range(&self) -> Option<FoldingRange> {
991-
let recasted = self.recast(&FormatOptions::default(), 0, crate::unparser::ExprContext::Other);
991+
let mut recasted = String::new();
992+
self.recast(
993+
&mut recasted,
994+
&FormatOptions::default(),
995+
0,
996+
crate::unparser::ExprContext::Other,
997+
);
992998
// If the code only has one line then we don't need to fold it.
993999
if recasted.lines().count() <= 1 {
9941000
return None;
@@ -2039,7 +2045,8 @@ impl From<&VariableDeclaration> for Vec<CompletionItem> {
20392045

20402046
impl Node<VariableDeclaration> {
20412047
pub fn get_lsp_folding_range(&self) -> Option<FoldingRange> {
2042-
let recasted = self.recast(&FormatOptions::default(), 0);
2048+
let mut recasted = String::new();
2049+
self.recast(&mut recasted, &FormatOptions::default(), 0);
20432050
// If the recasted value only has one line, don't fold it.
20442051
if recasted.lines().count() <= 1 {
20452052
return None;
@@ -3493,13 +3500,13 @@ impl FunctionExpression {
34933500
signature.push_str("()");
34943501
} else if self.params.len() == 1 {
34953502
signature.push('(');
3496-
signature.push_str(&self.params[0].recast(&FormatOptions::default(), 0));
3503+
self.params[0].recast(&mut signature, &FormatOptions::default(), 0);
34973504
signature.push(')');
34983505
} else {
34993506
signature.push('(');
35003507
for a in &self.params {
35013508
signature.push_str("\n ");
3502-
signature.push_str(&a.recast(&FormatOptions::default(), 0));
3509+
a.recast(&mut signature, &FormatOptions::default(), 0);
35033510
signature.push(',');
35043511
}
35053512
signature.push('\n');
@@ -3581,6 +3588,15 @@ impl FormatOptions {
35813588
}
35823589
}
35833590

3591+
/// Get the indentation string for the given level.
3592+
pub fn write_indentation(&self, buf: &mut String, times: usize) {
3593+
let ind = if self.use_tabs { '\t' } else { ' ' };
3594+
let n = if self.use_tabs { 1 } else { self.tab_size };
3595+
for _ in 0..(times * n) {
3596+
buf.push(ind);
3597+
}
3598+
}
3599+
35843600
/// Get the indentation string for the given level.
35853601
/// But offset the pipe operator (and a space) by one level.
35863602
pub fn get_indentation_offset_pipe(&self, level: usize) -> String {
@@ -4223,7 +4239,7 @@ startSketchOn(XY)"#;
42234239

42244240
assert_eq!(meta_settings.default_length_units, crate::execution::types::UnitLen::Mm);
42254241

4226-
let formatted = new_program.recast(&Default::default(), 0);
4242+
let formatted = new_program.recast_top(&Default::default(), 0);
42274243

42284244
assert_eq!(
42294245
formatted,
@@ -4252,7 +4268,7 @@ startSketchOn(XY)
42524268

42534269
assert_eq!(meta_settings.default_length_units, crate::execution::types::UnitLen::Mm);
42544270

4255-
let formatted = new_program.recast(&Default::default(), 0);
4271+
let formatted = new_program.recast_top(&Default::default(), 0);
42564272

42574273
assert_eq!(
42584274
formatted,
@@ -4287,7 +4303,7 @@ startSketchOn(XY)
42874303

42884304
assert_eq!(meta_settings.default_length_units, crate::execution::types::UnitLen::Cm);
42894305

4290-
let formatted = new_program.recast(&Default::default(), 0);
4306+
let formatted = new_program.recast_top(&Default::default(), 0);
42914307

42924308
assert_eq!(
42934309
formatted,
@@ -4341,7 +4357,7 @@ angle = atan(rise / run)"#;
43414357
program.rename_symbol("yoyo", var_decl.as_source_range().start() + 1);
43424358

43434359
// Recast the program to a string.
4344-
let formatted = program.recast(&Default::default(), 0);
4360+
let formatted = program.recast_top(&Default::default(), 0);
43454361

43464362
assert_eq!(
43474363
formatted,

rust/kcl-lib/src/simulation_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,7 +183,7 @@ async fn unparse_test(test: &Test) {
183183
let ast = crate::parsing::parse_tokens(tokens).unwrap();
184184

185185
// Check recasting.
186-
let actual = ast.recast(&Default::default(), 0);
186+
let actual = ast.recast_top(&Default::default(), 0);
187187
let input_result = catch_unwind(AssertUnwindSafe(|| {
188188
assert_snapshot(test, "Result of unparsing", || {
189189
insta::assert_snapshot!("unparsed", actual);

0 commit comments

Comments
 (0)