Skip to content

Commit 32799e7

Browse files
authored
perf: avoid file text string clone in wasm (#639)
1 parent c17a05e commit 32799e7

File tree

6 files changed

+48
-45
lines changed

6 files changed

+48
-45
lines changed

Cargo.lock

Lines changed: 22 additions & 22 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ harness = false
3131

3232
[dependencies]
3333
anyhow = "1.0.64"
34-
deno_ast = { version = "0.38.0", features = ["view"] }
34+
deno_ast = { version = "0.39.0", features = ["view"] }
3535
dprint-core = { version = "0.66.2", features = ["formatting"] }
3636
dprint-core-macros = "0.1.0"
3737
percent-encoding = "2.3.1"

src/format_text.rs

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
use std::path::Path;
2+
use std::sync::Arc;
23

34
use anyhow::Result;
45
use deno_ast::ParsedSource;
@@ -33,21 +34,24 @@ use super::swc::parse_swc_ast;
3334
///
3435
/// // now format many files (it is recommended to parallelize this)
3536
/// let files_to_format = vec![(PathBuf::from("path/to/file.ts"), "const t = 5 ;")];
36-
/// for (file_path, file_text) in files_to_format.iter() {
37-
/// let result = format_text(file_path, file_text, &config);
37+
/// for (file_path, file_text) in files_to_format {
38+
/// let result = format_text(&file_path, file_text.into(), &config);
3839
/// // save result here...
3940
/// }
4041
/// ```
41-
pub fn format_text(file_path: &Path, file_text: &str, config: &Configuration) -> Result<Option<String>> {
42-
if super::utils::file_text_has_ignore_comment(file_text, &config.ignore_file_comment_text) {
42+
pub fn format_text(file_path: &Path, file_text: String, config: &Configuration) -> Result<Option<String>> {
43+
if super::utils::file_text_has_ignore_comment(&file_text, &config.ignore_file_comment_text) {
4344
Ok(None)
4445
} else {
46+
let had_bom = file_text.starts_with("\u{FEFF}");
47+
let file_text = if had_bom { file_text[3..].to_string() } else { file_text };
48+
let file_text: Arc<str> = file_text.into();
4549
let parsed_source = parse_swc_ast(file_path, file_text)?;
4650
match inner_format(&parsed_source, config)? {
4751
Some(new_text) => Ok(Some(new_text)),
4852
None => {
49-
if let Some(stripped) = file_text.strip_prefix("\u{FEFF}") {
50-
Ok(Some(stripped.to_string()))
53+
if had_bom {
54+
Ok(Some(parsed_source.text().to_string()))
5155
} else {
5256
Ok(None)
5357
}
@@ -58,7 +62,7 @@ pub fn format_text(file_path: &Path, file_text: &str, config: &Configuration) ->
5862

5963
/// Formats an already parsed source. This is useful as a performance optimization.
6064
pub fn format_parsed_source(source: &ParsedSource, config: &Configuration) -> Result<Option<String>> {
61-
if super::utils::file_text_has_ignore_comment(source.text_info().text_str(), &config.ignore_file_comment_text) {
65+
if super::utils::file_text_has_ignore_comment(source.text(), &config.ignore_file_comment_text) {
6266
Ok(None)
6367
} else {
6468
ensure_no_specific_syntax_errors(source)?;
@@ -74,9 +78,9 @@ fn inner_format(parsed_source: &ParsedSource, config: &Configuration) -> Result<
7478
// println!("{}", print_items.get_as_text());
7579
print_items
7680
},
77-
config_to_print_options(parsed_source.text_info().text_str(), config),
81+
config_to_print_options(parsed_source.text(), config),
7882
);
79-
if result == parsed_source.text_info().text_str() {
83+
if result == parsed_source.text().as_ref() {
8084
Ok(None)
8185
} else {
8286
Ok(Some(result))
@@ -107,7 +111,7 @@ mod test {
107111
fn strips_bom() {
108112
for input_text in ["\u{FEFF}const t = 5;\n", "\u{FEFF}const t = 5;"] {
109113
let config = crate::configuration::ConfigurationBuilder::new().build();
110-
let result = format_text(&std::path::PathBuf::from("test.ts"), input_text, &config).unwrap().unwrap();
114+
let result = format_text(&std::path::PathBuf::from("test.ts"), input_text.into(), &config).unwrap().unwrap();
111115
assert_eq!(result, "const t = 5;\n");
112116
}
113117
}

src/swc.rs

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,10 @@ use deno_ast::swc::parser::error::SyntaxError;
55
use deno_ast::swc::parser::Syntax;
66
use deno_ast::ModuleSpecifier;
77
use deno_ast::ParsedSource;
8-
use deno_ast::SourceTextInfo;
98
use std::path::Path;
9+
use std::sync::Arc;
1010

11-
pub fn parse_swc_ast(file_path: &Path, file_text: &str) -> Result<ParsedSource> {
12-
let file_text = SourceTextInfo::from_string(file_text.to_string());
11+
pub fn parse_swc_ast(file_path: &Path, file_text: Arc<str>) -> Result<ParsedSource> {
1312
match parse_inner(file_path, file_text.clone()) {
1413
Ok(result) => Ok(result),
1514
Err(err) => {
@@ -28,13 +27,13 @@ pub fn parse_swc_ast(file_path: &Path, file_text: &str) -> Result<ParsedSource>
2827
}
2928
}
3029

31-
fn parse_inner(file_path: &Path, text_info: SourceTextInfo) -> Result<ParsedSource> {
32-
let parsed_source = parse_inner_no_diagnostic_check(file_path, text_info)?;
30+
fn parse_inner(file_path: &Path, text: Arc<str>) -> Result<ParsedSource> {
31+
let parsed_source = parse_inner_no_diagnostic_check(file_path, text)?;
3332
ensure_no_specific_syntax_errors(&parsed_source)?;
3433
Ok(parsed_source)
3534
}
3635

37-
fn parse_inner_no_diagnostic_check(file_path: &Path, text_info: SourceTextInfo) -> Result<ParsedSource> {
36+
fn parse_inner_no_diagnostic_check(file_path: &Path, text: Arc<str>) -> Result<ParsedSource> {
3837
let media_type = deno_ast::MediaType::from_path(file_path);
3938
let mut syntax = deno_ast::get_syntax(media_type);
4039
if let Syntax::Es(es) = &mut syntax {
@@ -47,7 +46,7 @@ fn parse_inner_no_diagnostic_check(file_path: &Path, text_info: SourceTextInfo)
4746
maybe_syntax: Some(syntax),
4847
media_type,
4948
scope_analysis: false,
50-
text_info,
49+
text,
5150
})
5251
.map_err(|diagnostic| anyhow!("{:#}", &diagnostic))
5352
}
@@ -247,7 +246,7 @@ mod tests {
247246

248247
fn run_fatal_diagnostic_test(file_path: &str, text: &str, expected: &str) {
249248
let file_path = PathBuf::from(file_path);
250-
assert_eq!(parse_swc_ast(&file_path, text).err().unwrap().to_string(), expected);
249+
assert_eq!(parse_swc_ast(&file_path, text.into()).err().unwrap().to_string(), expected);
251250
}
252251

253252
#[test]
@@ -343,11 +342,11 @@ Merge conflict marker encountered. at file:///test.ts:6:1
343342

344343
fn run_non_fatal_diagnostic_test(file_path: &str, text: &str, expected: &str) {
345344
let file_path = PathBuf::from(file_path);
346-
assert_eq!(format!("{}", parse_swc_ast(&file_path, text).err().unwrap()), expected);
345+
assert_eq!(format!("{}", parse_swc_ast(&file_path, text.into()).err().unwrap()), expected);
347346

348347
// this error should also be surfaced in `format_parsed_source` if someone provides
349348
// a source file that had a non-fatal diagnostic
350-
let parsed_source = parse_inner_no_diagnostic_check(&file_path, SourceTextInfo::from_string(text.to_string())).unwrap();
349+
let parsed_source = parse_inner_no_diagnostic_check(&file_path, text.into()).unwrap();
351350
let config = ConfigurationBuilder::new().build();
352351
assert_eq!(crate::format_parsed_source(&parsed_source, &config).err().unwrap().to_string(), expected);
353352
}

src/wasm_plugin.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ impl SyncPluginHandler<Configuration> for TypeScriptPluginHandler {
5858
_format_with_host: impl FnMut(&Path, Vec<u8>, &ConfigKeyMap) -> FormatResult,
5959
) -> FormatResult {
6060
let file_text = String::from_utf8(file_bytes)?;
61-
super::format_text(file_path, &file_text, config).map(|maybe_text| maybe_text.map(|t| t.into_bytes()))
61+
super::format_text(file_path, file_text, config).map(|maybe_text| maybe_text.map(|t| t.into_bytes()))
6262
}
6363
}
6464

tests/spec_test.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ fn main() {
2828
let config_result = resolve_config(spec_config, &global_config);
2929
ensure_no_diagnostics(&config_result.diagnostics);
3030

31-
format_text(file_name, file_text, &config_result.config)
31+
format_text(file_name, file_text.into(), &config_result.config)
3232
})
3333
},
3434
Arc::new(move |_file_name, _file_text, _spec_config| {

0 commit comments

Comments
 (0)