From 342c81cd8a58c468c13839ec42b625319008b8b5 Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 25 Oct 2024 17:27:44 -0400 Subject: [PATCH 1/2] perf: pass transpiled modules to deno_core as a string --- cli/cache/emit.rs | 25 +++++++------------------ cli/emit.rs | 17 +++++++++-------- cli/module_loader.rs | 6 ++++-- cli/standalone/binary.rs | 2 +- cli/tools/coverage/mod.rs | 5 ++--- 5 files changed, 23 insertions(+), 32 deletions(-) diff --git a/cli/cache/emit.rs b/cli/cache/emit.rs index 6807f06c1063e4..74e1c1101bb6fd 100644 --- a/cli/cache/emit.rs +++ b/cli/cache/emit.rs @@ -39,7 +39,7 @@ impl EmitCache { &self, specifier: &ModuleSpecifier, expected_source_hash: u64, - ) -> Option> { + ) -> Option { let emit_filename = self.get_emit_filename(specifier)?; let bytes = self.disk_cache.get(&emit_filename).ok()?; self @@ -100,7 +100,7 @@ impl EmitFileSerializer { &self, mut bytes: Vec, expected_source_hash: u64, - ) -> Option> { + ) -> Option { let last_newline_index = bytes.iter().rposition(|&b| b == b'\n')?; let (content, last_line) = bytes.split_at(last_newline_index); let hashes = last_line.strip_prefix(LAST_LINE_PREFIX.as_bytes())?; @@ -120,7 +120,7 @@ impl EmitFileSerializer { // everything looks good, truncate and return it bytes.truncate(content.len()); - Some(bytes) + String::from_utf8(bytes).ok() } pub fn serialize(&self, code: &[u8], source_hash: u64) -> Vec { @@ -170,8 +170,6 @@ mod test { }, emit_failed_flag: Default::default(), }; - let to_string = - |bytes: Vec| -> String { String::from_utf8(bytes).unwrap() }; let specifier1 = ModuleSpecifier::from_file_path(temp_dir.path().join("file1.ts")) @@ -188,13 +186,10 @@ mod test { assert_eq!(cache.get_emit_code(&specifier1, 5), None); // providing the correct source hash assert_eq!( - cache.get_emit_code(&specifier1, 10).map(to_string), + cache.get_emit_code(&specifier1, 10), Some(emit_code1.clone()), ); - assert_eq!( - cache.get_emit_code(&specifier2, 2).map(to_string), - Some(emit_code2) - ); + assert_eq!(cache.get_emit_code(&specifier2, 2), Some(emit_code2)); // try changing the cli version (should not load previous ones) let cache = EmitCache { @@ -215,18 +210,12 @@ mod test { }, emit_failed_flag: Default::default(), }; - assert_eq!( - cache.get_emit_code(&specifier1, 5).map(to_string), - Some(emit_code1) - ); + assert_eq!(cache.get_emit_code(&specifier1, 5), Some(emit_code1)); // adding when already exists should not cause issue let emit_code3 = "asdf".to_string(); cache.set_emit_code(&specifier1, 20, emit_code3.as_bytes()); assert_eq!(cache.get_emit_code(&specifier1, 5), None); - assert_eq!( - cache.get_emit_code(&specifier1, 20).map(to_string), - Some(emit_code3) - ); + assert_eq!(cache.get_emit_code(&specifier1, 20), Some(emit_code3)); } } diff --git a/cli/emit.rs b/cli/emit.rs index ad200af0504222..9cb407f9e9c01f 100644 --- a/cli/emit.rs +++ b/cli/emit.rs @@ -13,7 +13,6 @@ use deno_core::error::AnyError; use deno_core::futures::stream::FuturesUnordered; use deno_core::futures::FutureExt; use deno_core::futures::StreamExt; -use deno_core::ModuleCodeBytes; use deno_core::ModuleSpecifier; use deno_graph::MediaType; use deno_graph::Module; @@ -94,7 +93,7 @@ impl Emitter { &self, specifier: &ModuleSpecifier, source: &str, - ) -> Option> { + ) -> Option { let source_hash = self.get_source_hash(source); self.emit_cache.get_emit_code(specifier, source_hash) } @@ -104,7 +103,7 @@ impl Emitter { specifier: &ModuleSpecifier, media_type: MediaType, source: &Arc, - ) -> Result { + ) -> Result { // Note: keep this in sync with the sync version below let helper = EmitParsedSourceHelper(self); match helper.pre_emit_parsed_source(specifier, source) { @@ -143,7 +142,7 @@ impl Emitter { specifier: &ModuleSpecifier, media_type: MediaType, source: &Arc, - ) -> Result { + ) -> Result { // Note: keep this in sync with the async version above let helper = EmitParsedSourceHelper(self); match helper.pre_emit_parsed_source(specifier, source) { @@ -227,7 +226,7 @@ impl Emitter { } enum PreEmitResult { - Cached(ModuleCodeBytes), + Cached(String), NotCached { source_hash: u64 }, } @@ -245,7 +244,7 @@ impl<'a> EmitParsedSourceHelper<'a> { if let Some(emit_code) = self.0.emit_cache.get_emit_code(specifier, source_hash) { - PreEmitResult::Cached(emit_code.into_boxed_slice().into()) + PreEmitResult::Cached(emit_code) } else { PreEmitResult::NotCached { source_hash } } @@ -272,7 +271,7 @@ impl<'a> EmitParsedSourceHelper<'a> { specifier: &ModuleSpecifier, transpile_result: TranspileResult, source_hash: u64, - ) -> ModuleCodeBytes { + ) -> String { let transpiled_source = match transpile_result { TranspileResult::Owned(source) => source, TranspileResult::Cloned(source) => { @@ -286,7 +285,9 @@ impl<'a> EmitParsedSourceHelper<'a> { source_hash, &transpiled_source.source, ); - transpiled_source.source.into_boxed_slice().into() + // todo(https://github.com/denoland/deno_ast/issues/282): move to deno_ast + // SAFETY: This is fine because swc is working off of Strings + unsafe { String::from_utf8_unchecked(transpiled_source.source) } } } diff --git a/cli/module_loader.rs b/cli/module_loader.rs index 37d42f78e5e93b..4a020516e70a59 100644 --- a/cli/module_loader.rs +++ b/cli/module_loader.rs @@ -541,7 +541,8 @@ impl self.parsed_source_cache.free(specifier); Ok(Some(ModuleCodeStringSource { - code: ModuleSourceCode::Bytes(transpile_result), + // note: it's faster to provide a string if we know it's a string + code: ModuleSourceCode::String(transpile_result.into()), found_url: specifier.clone(), media_type, })) @@ -571,7 +572,8 @@ impl self.parsed_source_cache.free(specifier); Ok(Some(ModuleCodeStringSource { - code: ModuleSourceCode::Bytes(transpile_result), + // note: it's faster to provide a string if we know it's a string + code: ModuleSourceCode::String(transpile_result.into()), found_url: specifier.clone(), media_type, })) diff --git a/cli/standalone/binary.rs b/cli/standalone/binary.rs index 0f8b0b49d4a408..f41f3003ff1b3e 100644 --- a/cli/standalone/binary.rs +++ b/cli/standalone/binary.rs @@ -613,7 +613,7 @@ impl<'a> DenoCompileBinaryWriter<'a> { .emitter .emit_parsed_source(&m.specifier, m.media_type, &m.source) .await?; - source.to_vec() + source.into_bytes() } else { m.source.as_bytes().to_vec() }; diff --git a/cli/tools/coverage/mod.rs b/cli/tools/coverage/mod.rs index 260c0c842477a7..3b08f2c77adc90 100644 --- a/cli/tools/coverage/mod.rs +++ b/cli/tools/coverage/mod.rs @@ -571,7 +571,7 @@ pub async fn cover_files( | MediaType::Cjs | MediaType::Mjs | MediaType::Json => None, - MediaType::Dts | MediaType::Dmts | MediaType::Dcts => Some(Vec::new()), + MediaType::Dts | MediaType::Dmts | MediaType::Dcts => Some(String::new()), MediaType::TypeScript | MediaType::Jsx | MediaType::Mts @@ -593,8 +593,7 @@ pub async fn cover_files( } }; let runtime_code: String = match transpiled_code { - Some(code) => String::from_utf8(code) - .with_context(|| format!("Failed decoding {}", file.specifier))?, + Some(code) => code, None => original_source.to_string(), }; From 3cd7c94b722d88ac83289f7c88cec4c0ab90ffda Mon Sep 17 00:00:00 2001 From: David Sherret Date: Fri, 25 Oct 2024 17:34:01 -0400 Subject: [PATCH 2/2] update --- cli/emit.rs | 46 ++++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/cli/emit.rs b/cli/emit.rs index 9cb407f9e9c01f..8e93092e67b1cd 100644 --- a/cli/emit.rs +++ b/cli/emit.rs @@ -112,7 +112,7 @@ impl Emitter { let parsed_source_cache = self.parsed_source_cache.clone(); let transpile_and_emit_options = self.transpile_and_emit_options.clone(); - let transpile_result = deno_core::unsync::spawn_blocking({ + let transpiled_source = deno_core::unsync::spawn_blocking({ let specifier = specifier.clone(); let source = source.clone(); move || -> Result<_, AnyError> { @@ -128,11 +128,12 @@ impl Emitter { }) .await .unwrap()?; - Ok(helper.post_emit_parsed_source( + helper.post_emit_parsed_source( specifier, - transpile_result, + &transpiled_source, source_hash, - )) + ); + Ok(transpiled_source) } } } @@ -148,7 +149,7 @@ impl Emitter { match helper.pre_emit_parsed_source(specifier, source) { PreEmitResult::Cached(emitted_text) => Ok(emitted_text), PreEmitResult::NotCached { source_hash } => { - let transpile_result = EmitParsedSourceHelper::transpile( + let transpiled_source = EmitParsedSourceHelper::transpile( &self.parsed_source_cache, specifier, source.clone(), @@ -156,11 +157,12 @@ impl Emitter { &self.transpile_and_emit_options.0, &self.transpile_and_emit_options.1, )?; - Ok(helper.post_emit_parsed_source( + helper.post_emit_parsed_source( specifier, - transpile_result, + &transpiled_source, source_hash, - )) + ); + Ok(transpiled_source) } } } @@ -257,21 +259,14 @@ impl<'a> EmitParsedSourceHelper<'a> { media_type: MediaType, transpile_options: &deno_ast::TranspileOptions, emit_options: &deno_ast::EmitOptions, - ) -> Result { + ) -> Result { // nothing else needs the parsed source at this point, so remove from // the cache in order to not transpile owned let parsed_source = parsed_source_cache .remove_or_parse_module(specifier, source, media_type)?; ensure_no_import_assertion(&parsed_source)?; - Ok(parsed_source.transpile(transpile_options, emit_options)?) - } - - pub fn post_emit_parsed_source( - &self, - specifier: &ModuleSpecifier, - transpile_result: TranspileResult, - source_hash: u64, - ) -> String { + let transpile_result = + parsed_source.transpile(transpile_options, emit_options)?; let transpiled_source = match transpile_result { TranspileResult::Owned(source) => source, TranspileResult::Cloned(source) => { @@ -280,14 +275,21 @@ impl<'a> EmitParsedSourceHelper<'a> { } }; debug_assert!(transpiled_source.source_map.is_none()); + let text = String::from_utf8(transpiled_source.source)?; + Ok(text) + } + + pub fn post_emit_parsed_source( + &self, + specifier: &ModuleSpecifier, + transpiled_source: &str, + source_hash: u64, + ) { self.0.emit_cache.set_emit_code( specifier, source_hash, - &transpiled_source.source, + transpiled_source.as_bytes(), ); - // todo(https://github.com/denoland/deno_ast/issues/282): move to deno_ast - // SAFETY: This is fine because swc is working off of Strings - unsafe { String::from_utf8_unchecked(transpiled_source.source) } } }