From 1927f2cf7d32fceef2c6a4c1e3b1ccf617dc0546 Mon Sep 17 00:00:00 2001 From: Nathan Whitaker <17734409+nathanwhit@users.noreply.github.com> Date: Mon, 10 Feb 2025 15:06:38 +0100 Subject: [PATCH] feat: optional code cache for extension sources (#1071) Wires up the code cache through the `ExtModuleLoader` and extension initialization, so that embedders can use the V8 code cache on extension sources. --- Cargo.lock | 6 +-- Cargo.toml | 2 +- core/error.rs | 6 ++- core/inspector.rs | 2 +- core/lib.rs | 1 + core/modules/loaders.rs | 51 +++++++++++++++++++--- core/modules/mod.rs | 1 + core/modules/tests.rs | 2 +- core/runtime/jsrealm.rs | 89 +++++++++++++++++++++++++++++++++++++++ core/runtime/jsruntime.rs | 48 ++++++++++++++++++--- 10 files changed, 189 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a164ff842..b76cd88ff 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1409,7 +1409,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "e310b3a6b5907f99202fcdb4960ff45b93735d7c7d96b760fcff8db2dc0e103d" dependencies = [ "cfg-if", - "windows-targets 0.52.5", + "windows-targets 0.48.5", ] [[package]] @@ -3058,9 +3058,9 @@ dependencies = [ [[package]] name = "v8" -version = "130.0.8" +version = "130.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "be16314fd485983a2a2e001d90a959d6c7c3eb800a2f481b11104f76cd5608cd" +checksum = "a511192602f7b435b0a241c1947aa743eb7717f20a9195f4b5e8ed1952e01db1" dependencies = [ "bindgen", "bitflags", diff --git a/Cargo.toml b/Cargo.toml index 309a9adcf..b9d7aedf8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -29,7 +29,7 @@ deno_ast = { version = "=0.44.0", features = ["transpiling"] } deno_core_icudata = "0.74.0" deno_error = { version = "0.5.5", features = ["serde_json", "serde", "url", "tokio"] } deno_unsync = "0.4.1" -v8 = { version = "130.0.8", default-features = false } +v8 = { version = "130.0.7", default-features = false } anyhow = "1" bencher = "0.1" diff --git a/core/error.rs b/core/error.rs index 70d651fcf..4f3552f5e 100644 --- a/core/error.rs +++ b/core/error.rs @@ -81,6 +81,8 @@ pub enum CoreError { Module(ModuleConcreteError), #[error(transparent)] DataError(DataError), + #[error("Unable to get code cache from unbound module script for {0}")] + CreateCodeCache(String), } impl CoreError { @@ -174,6 +176,7 @@ impl JsErrorClass for CoreError { | CoreError::MissingFromModuleMap(_) | CoreError::ExecutionTerminated | CoreError::PendingPromiseResolution + | CoreError::CreateCodeCache(_) | CoreError::EvaluateDynamicImportedModule => { Cow::Borrowed(GENERIC_ERROR) } @@ -206,7 +209,8 @@ impl JsErrorClass for CoreError { | CoreError::FutureCanceled(_) | CoreError::ExecutionTerminated | CoreError::PendingPromiseResolution - | CoreError::EvaluateDynamicImportedModule => self.to_string().into(), + | CoreError::EvaluateDynamicImportedModule + | CoreError::CreateCodeCache(_) => self.to_string().into(), } } diff --git a/core/inspector.rs b/core/inspector.rs index a68aac50b..0cb58502d 100644 --- a/core/inspector.rs +++ b/core/inspector.rs @@ -258,7 +258,7 @@ impl JsRuntimeInspector { let stack_trace = message.get_stack_trace(scope); let mut v8_inspector_ref = self.v8_inspector.borrow_mut(); let v8_inspector = v8_inspector_ref.as_mut().unwrap(); - let stack_trace = v8_inspector.create_stack_trace(stack_trace); + let stack_trace = v8_inspector.create_stack_trace(stack_trace.unwrap()); v8_inspector.exception_thrown( context, if in_promise { diff --git a/core/lib.rs b/core/lib.rs index ba86e22a4..9b195eced 100644 --- a/core/lib.rs +++ b/core/lib.rs @@ -107,6 +107,7 @@ pub use crate::module_specifier::resolve_url; pub use crate::module_specifier::ModuleResolutionError; pub use crate::module_specifier::ModuleSpecifier; pub use crate::modules::CustomModuleEvaluationKind; +pub use crate::modules::ExtCodeCache; pub use crate::modules::FsModuleLoader; pub use crate::modules::ModuleCodeBytes; pub use crate::modules::ModuleCodeString; diff --git a/core/modules/loaders.rs b/core/modules/loaders.rs index 70724e699..873a7842f 100644 --- a/core/modules/loaders.rs +++ b/core/modules/loaders.rs @@ -1,5 +1,6 @@ // Copyright 2018-2025 the Deno authors. MIT license. +use crate::error::CoreError; use crate::module_specifier::ModuleSpecifier; use crate::modules::IntoModuleCodeString; use crate::modules::ModuleCodeString; @@ -13,7 +14,6 @@ use crate::resolve_import; use crate::ModuleSourceCode; use deno_error::JsErrorBox; -use deno_core::error::CoreError; use futures::future::FutureExt; use std::borrow::Cow; use std::cell::RefCell; @@ -22,6 +22,8 @@ use std::future::Future; use std::pin::Pin; use std::rc::Rc; +use super::SourceCodeCacheInfo; + #[derive(Debug, thiserror::Error, deno_error::JsError)] #[class(generic)] pub enum ModuleLoaderError { @@ -220,12 +222,33 @@ impl ModuleLoader for NoopModuleLoader { } } +pub trait ExtCodeCache { + fn get_code_cache_info( + &self, + specifier: &ModuleSpecifier, + code: &ModuleSourceCode, + esm: bool, + ) -> SourceCodeCacheInfo; + + fn code_cache_ready( + &self, + specifier: ModuleSpecifier, + hash: u64, + code_cache: &[u8], + esm: bool, + ); +} + pub(crate) struct ExtModuleLoader { sources: RefCell>, + ext_code_cache: Option>, } impl ExtModuleLoader { - pub fn new(loaded_sources: Vec<(ModuleName, ModuleCodeString)>) -> Self { + pub fn new( + loaded_sources: Vec<(ModuleName, ModuleCodeString)>, + ext_code_cache: Option>, + ) -> Self { // Guesstimate a length let mut sources = HashMap::with_capacity(loaded_sources.len()); for source in loaded_sources { @@ -233,10 +256,11 @@ impl ExtModuleLoader { } ExtModuleLoader { sources: RefCell::new(sources), + ext_code_cache, } } - pub fn finalize(self) -> Result<(), CoreError> { + pub fn finalize(&self) -> Result<(), CoreError> { let sources = self.sources.take(); let unused_modules: Vec<_> = sources.iter().collect(); @@ -296,11 +320,16 @@ impl ModuleLoader for ExtModuleLoader { )) } }; + let code = ModuleSourceCode::String(source); + let code_cache = self + .ext_code_cache + .as_ref() + .map(|cache| cache.get_code_cache_info(specifier, &code, true)); ModuleLoadResponse::Sync(Ok(ModuleSource::new( ModuleType::JavaScript, - ModuleSourceCode::String(source), + code, specifier, - None, + code_cache, ))) } @@ -312,6 +341,18 @@ impl ModuleLoader for ExtModuleLoader { ) -> Pin>>> { async { Ok(()) }.boxed_local() } + + fn code_cache_ready( + &self, + module_specifier: ModuleSpecifier, + hash: u64, + code_cache: &[u8], + ) -> Pin>> { + if let Some(ext_code_cache) = &self.ext_code_cache { + ext_code_cache.code_cache_ready(module_specifier, hash, code_cache, true); + } + std::future::ready(()).boxed_local() + } } /// A loader that is used in `op_lazy_load_esm` to load and execute diff --git a/core/modules/mod.rs b/core/modules/mod.rs index a879f92be..10435044f 100644 --- a/core/modules/mod.rs +++ b/core/modules/mod.rs @@ -21,6 +21,7 @@ mod recursive_load; #[cfg(all(test, not(miri)))] mod tests; +pub use loaders::ExtCodeCache; pub(crate) use loaders::ExtModuleLoader; pub use loaders::FsModuleLoader; pub(crate) use loaders::LazyEsmModuleLoader; diff --git a/core/modules/tests.rs b/core/modules/tests.rs index 6cb893583..d7a60b5ce 100644 --- a/core/modules/tests.rs +++ b/core/modules/tests.rs @@ -1946,7 +1946,7 @@ fn test_load_with_code_cache() { #[test] fn ext_module_loader_relative() { - let loader = ExtModuleLoader::new(vec![]); + let loader = ExtModuleLoader::new(vec![], None); let cases = [ ( ("../../foo.js", "ext:test/nested/mod/bar.js"), diff --git a/core/runtime/jsrealm.rs b/core/runtime/jsrealm.rs index 4122414b1..ced9d533e 100644 --- a/core/runtime/jsrealm.rs +++ b/core/runtime/jsrealm.rs @@ -3,6 +3,8 @@ use super::exception_state::ExceptionState; #[cfg(test)] use super::op_driver::OpDriver; +use crate::ModuleSourceCode; +use crate::SourceCodeCacheInfo; use crate::_ops::OpMethodDecl; use crate::cppgc::FunctionTemplateData; use crate::error::exception_to_err_result; @@ -352,6 +354,93 @@ impl JsRealm { } } + // TODO(nathanwhit): reduce duplication between this and `execute_script`, and + // try to factor out the code cache logic to share with `op_eval_context` + pub fn execute_script_with_cache( + &self, + isolate: &mut v8::Isolate, + name: ModuleSpecifier, + source_code: impl IntoModuleCodeString, + get_cache: &dyn Fn( + &ModuleSpecifier, + &ModuleSourceCode, + ) -> SourceCodeCacheInfo, + cache_ready: &dyn Fn(ModuleSpecifier, u64, &[u8]), + ) -> Result, CoreError> { + let scope = &mut self.0.handle_scope(isolate); + + let specifier = name.clone(); + let code = source_code.into_module_code(); + let source = ModuleSourceCode::String(code); + let code_cache = get_cache(&name, &source); + let ModuleSourceCode::String(source) = source else { + unreachable!() + }; + let name = name.into_module_name().v8_string(scope).unwrap(); + let source = source.v8_string(scope).unwrap(); + let origin = script_origin(scope, name, false, None); + let tc_scope = &mut v8::TryCatch::new(scope); + + let (maybe_script, maybe_code_cache_hash) = + if let Some(data) = &code_cache.data { + let mut source = v8::script_compiler::Source::new_with_cached_data( + source, + Some(&origin), + v8::CachedData::new(data), + ); + let script = v8::script_compiler::compile( + tc_scope, + &mut source, + v8::script_compiler::CompileOptions::ConsumeCodeCache, + v8::script_compiler::NoCacheReason::NoReason, + ); + // Check if the provided code cache is rejected by V8. + let rejected = match source.get_cached_data() { + Some(cached_data) => cached_data.rejected(), + _ => true, + }; + let maybe_code_cache_hash = if rejected { + Some(code_cache.hash) // recreate the cache + } else { + None + }; + (Some(script), maybe_code_cache_hash) + } else { + (None, Some(code_cache.hash)) + }; + + let script = maybe_script + .unwrap_or_else(|| v8::Script::compile(tc_scope, source, Some(&origin))); + + let script = match script { + Some(script) => script, + None => { + let exception = tc_scope.exception().unwrap(); + return exception_to_err_result(tc_scope, exception, false, false); + } + }; + + if let Some(code_cache_hash) = maybe_code_cache_hash { + let unbound_script = script.get_unbound_script(tc_scope); + let code_cache = unbound_script + .create_code_cache() + .ok_or_else(|| CoreError::CreateCodeCache(specifier.to_string()))?; + cache_ready(specifier, code_cache_hash, &code_cache); + } + + match script.run(tc_scope) { + Some(value) => { + let value_handle = v8::Global::new(tc_scope, value); + Ok(value_handle) + } + None => { + assert!(tc_scope.has_caught()); + let exception = tc_scope.exception().unwrap(); + exception_to_err_result(tc_scope, exception, false, false) + } + } + } + /// Returns the namespace object of a module. /// /// This is only available after module evaluation has completed. diff --git a/core/runtime/jsruntime.rs b/core/runtime/jsruntime.rs index cafbce00e..9318d3a01 100644 --- a/core/runtime/jsruntime.rs +++ b/core/runtime/jsruntime.rs @@ -30,6 +30,7 @@ use crate::modules::script_origin; use crate::modules::CustomModuleEvaluationCb; use crate::modules::EvalContextCodeCacheReadyCb; use crate::modules::EvalContextGetCodeCacheCb; +use crate::modules::ExtCodeCache; use crate::modules::ExtModuleLoader; use crate::modules::ImportMetaResolveCallback; use crate::modules::IntoModuleCodeString; @@ -450,6 +451,9 @@ pub struct RuntimeOptions { /// executed tries to load modules. pub module_loader: Option>, + /// If specified, enables V8 code cache for extension code. + pub extension_code_cache: Option>, + /// If specified, transpiles extensions before loading. pub extension_transpiler: Option>, @@ -1181,7 +1185,12 @@ impl JsRuntime { js_runtime.store_js_callbacks(&realm, will_snapshot); - js_runtime.init_extension_js(&realm, &module_map, sources)?; + js_runtime.init_extension_js( + &realm, + &module_map, + sources, + options.extension_code_cache, + )?; } if will_snapshot { @@ -1326,6 +1335,7 @@ impl JsRuntime { realm: &JsRealm, module_map: &Rc, loaded_sources: LoadedSources, + ext_code_cache: Option>, ) -> Result<(), CoreError> { // First, add all the lazy ESM for source in loaded_sources.lazy_esm { @@ -1344,11 +1354,13 @@ impl JsRuntime { modules.push(ModuleSpecifier::parse(&esm.specifier).unwrap()); sources.push((esm.specifier, esm.code)); } - let ext_loader = Rc::new(ExtModuleLoader::new(sources)); + let ext_loader = + Rc::new(ExtModuleLoader::new(sources, ext_code_cache.clone())); *module_map.loader.borrow_mut() = ext_loader.clone(); // Next, load the extension modules as side modules (but do not execute them) for module in modules { + // eprintln!("loading module: {module}"); realm .load_side_es_module_from_code(self.v8_isolate(), &module, None) .await?; @@ -1356,7 +1368,26 @@ impl JsRuntime { // Execute extension scripts for source in loaded_sources.js { - realm.execute_script(self.v8_isolate(), source.specifier, source.code)?; + if let Some(ext_code_cache) = &ext_code_cache { + let specifier = ModuleSpecifier::parse(&source.specifier)?; + realm.execute_script_with_cache( + self.v8_isolate(), + specifier, + source.code, + &|specifier, code| { + ext_code_cache.get_code_cache_info(specifier, code, false) + }, + &|specifier, hash, code_cache| { + ext_code_cache.code_cache_ready(specifier, hash, code_cache, false) + }, + )?; + } else { + realm.execute_script( + self.v8_isolate(), + source.specifier, + source.code, + )?; + } } // ...then execute all entry points @@ -1370,6 +1401,10 @@ impl JsRuntime { let isolate = self.v8_isolate(); let scope = &mut realm.handle_scope(isolate); module_map.mod_evaluate_sync(scope, mod_id)?; + let mut cx = Context::from_waker(futures::task::noop_waker_ref()); + // poll once so code cache is populated. the `ExtCodeCache` trait is sync, so + // the `CodeCacheReady` futures will always finish on the first poll. + let _ = module_map.poll_progress(&mut cx, scope); } #[cfg(debug_assertions)] @@ -1380,10 +1415,7 @@ impl JsRuntime { let module_map = realm.0.module_map(); *module_map.loader.borrow_mut() = loader; - Rc::try_unwrap(ext_loader) - .map_err(drop) - .unwrap() - .finalize()?; + ext_loader.finalize()?; Ok(()) } @@ -1394,11 +1426,13 @@ impl JsRuntime { realm: &JsRealm, module_map: &Rc, loaded_sources: LoadedSources, + ext_code_cache: Option>, ) -> Result<(), CoreError> { futures::executor::block_on(self.init_extension_js_inner( realm, module_map, loaded_sources, + ext_code_cache, ))?; Ok(())