Skip to content

Commit

Permalink
refactor: pass hash into code cache (#752)
Browse files Browse the repository at this point in the history
This avoids the cli having to re-lookup the hash again.
  • Loading branch information
dsherret committed May 23, 2024
1 parent e9643e7 commit f0a7357
Show file tree
Hide file tree
Showing 9 changed files with 78 additions and 45 deletions.
12 changes: 12 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ strum_macros = "0.25.0"
testing_macros = "0.2.11"
thiserror = "1"
tokio = { version = "1", features = ["full"] }
twox-hash = "1.6.3"
url = { version = "2", features = ["serde", "expose_internals"] }

# macros
Expand Down
1 change: 1 addition & 0 deletions core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ deno_ast.workspace = true
fastrand.workspace = true
pretty_assertions.workspace = true
rstest.workspace = true
twox-hash.workspace = true
unicycle = "0"

[[bench]]
Expand Down
1 change: 1 addition & 0 deletions core/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ pub use crate::modules::ModuleLoader;
pub use crate::modules::ModuleName;
pub use crate::modules::ModuleSource;
pub use crate::modules::ModuleSourceCode;
pub use crate::modules::ModuleSourceCodeCache;
pub use crate::modules::ModuleSourceFuture;
pub use crate::modules::ModuleType;
pub use crate::modules::NoopModuleLoader;
Expand Down
3 changes: 2 additions & 1 deletion core/modules/loaders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,8 @@ pub trait ModuleLoader {
/// It's not required to implement this method.
fn code_cache_ready(
&self,
_module_specifier: &ModuleSpecifier,
_module_specifier: ModuleSpecifier,
_hash: u64,
_code_cache: &[u8],
) -> Pin<Box<dyn Future<Output = ()>>> {
async {}.boxed_local()
Expand Down
56 changes: 27 additions & 29 deletions core/modules/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ impl ModEvaluate {
}

type CodeCacheReadyCallback =
Box<dyn Fn(&[u8]) -> Pin<Box<dyn Future<Output = ()>>>>;
Box<dyn FnOnce(&[u8]) -> Pin<Box<dyn Future<Output = ()>>>>;
pub(crate) struct CodeCacheInfo {
code_cache: Option<Cow<'static, [u8]>>,
data: Option<Cow<'static, [u8]>>,
ready_callback: CodeCacheReadyCallback,
}

Expand Down Expand Up @@ -139,7 +139,6 @@ pub(crate) struct ModuleMap {
pending_code_cache_ready: Cell<bool>,
module_waker: AtomicWaker,
data: RefCell<ModuleMapData>,
enable_code_cache: bool,

/// A counter used to delay our dynamic import deadlock detection by one spin
/// of the event loop.
Expand Down Expand Up @@ -193,7 +192,6 @@ impl ModuleMap {
loader: Rc<dyn ModuleLoader>,
exception_state: Rc<ExceptionState>,
import_meta_resolve_cb: ImportMetaResolveCallback,
enable_code_cache: bool,
) -> Self {
Self {
loader: loader.into(),
Expand All @@ -212,7 +210,6 @@ impl ModuleMap {
pending_code_cache_ready: Default::default(),
module_waker: Default::default(),
data: Default::default(),
enable_code_cache,
}
}

Expand Down Expand Up @@ -346,24 +343,25 @@ impl ModuleMap {
ModuleSource::get_string_source(module_url_found.as_str(), code)
.map_err(ModuleError::Other)?;

let (code_cache_info, module_url_found) = if self.enable_code_cache {
let (module_url_found1, module_url_found2) =
module_url_found.into_cheap_copy();
let loader = self.loader.borrow().clone();
(
Some(CodeCacheInfo {
code_cache,
ready_callback: Box::new(move |cache| {
let specifier =
ModuleSpecifier::parse(module_url_found1.as_str()).unwrap();
loader.code_cache_ready(&specifier, cache)
let (code_cache_info, module_url_found) =
if let Some(code_cache) = code_cache {
let (module_url_found1, module_url_found2) =
module_url_found.into_cheap_copy();
let loader = self.loader.borrow().clone();
(
Some(CodeCacheInfo {
data: code_cache.data,
ready_callback: Box::new(move |cache| {
let specifier =
ModuleSpecifier::parse(module_url_found1.as_str()).unwrap();
loader.code_cache_ready(specifier, code_cache.hash, cache)
}),
}),
}),
module_url_found2,
)
} else {
(None, module_url_found)
};
module_url_found2,
)
} else {
(None, module_url_found)
};

self.new_module_from_js_source(
scope,
Expand Down Expand Up @@ -441,16 +439,16 @@ impl ModuleMap {
exports,
)?;

let (code_cache_info, url2) = if self.enable_code_cache {
let (code_cache_info, url2) = if let Some(code_cache) = code_cache {
let (url1, url2) = url2.into_cheap_copy();
let loader = self.loader.borrow().clone();
(
Some(CodeCacheInfo {
code_cache,
data: code_cache.data,
ready_callback: Box::new(move |cache| {
let specifier =
ModuleSpecifier::parse(url1.as_str()).unwrap();
loader.code_cache_ready(&specifier, cache)
loader.code_cache_ready(specifier, code_cache.hash, cache)
}),
}),
url2,
Expand Down Expand Up @@ -599,7 +597,7 @@ impl ModuleMap {
let (maybe_module, try_store_code_cache) = code_cache_info
.as_ref()
.and_then(|code_cache_info| {
code_cache_info.code_cache.as_ref().map(|cache| {
code_cache_info.data.as_ref().map(|cache| {
let mut source = v8::script_compiler::Source::new_with_cached_data(
source_str,
Some(&origin),
Expand Down Expand Up @@ -1743,12 +1741,12 @@ impl ModuleMap {
scope,
module_specifier,
ModuleSource::get_string_source(specifier.as_str(), source.code)?,
if self.enable_code_cache {
if let Some(code_cache) = source.code_cache {
let loader = self.loader.borrow().clone();
Some(CodeCacheInfo {
code_cache: source.code_cache,
data: code_cache.data,
ready_callback: Box::new(move |cache| {
loader.code_cache_ready(&specifier, cache)
loader.code_cache_ready(specifier, code_cache.hash, cache)
}),
})
} else {
Expand Down
14 changes: 10 additions & 4 deletions core/modules/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,12 @@ impl ModuleType {
}
}

#[derive(Debug)]
pub struct ModuleSourceCodeCache {
pub hash: u64,
pub data: Option<Cow<'static, [u8]>>,
}

/// EsModule source code that will be loaded into V8.
///
/// Users can implement `Into<ModuleInfo>` for different file types that
Expand All @@ -357,7 +363,7 @@ impl ModuleType {
pub struct ModuleSource {
pub code: ModuleSourceCode,
pub module_type: ModuleType,
pub code_cache: Option<Cow<'static, [u8]>>,
pub code_cache: Option<ModuleSourceCodeCache>,
module_url_specified: ModuleName,
/// If the module was found somewhere other than the specified address, this will be [`Some`].
module_url_found: Option<ModuleName>,
Expand All @@ -369,7 +375,7 @@ impl ModuleSource {
module_type: impl Into<ModuleType>,
code: ModuleSourceCode,
specifier: &ModuleSpecifier,
code_cache: Option<Cow<'static, [u8]>>,
code_cache: Option<ModuleSourceCodeCache>,
) -> Self {
let module_url_specified = specifier.as_ref().to_owned().into();
Self {
Expand All @@ -388,7 +394,7 @@ impl ModuleSource {
code: ModuleSourceCode,
specifier: &ModuleSpecifier,
specifier_found: &ModuleSpecifier,
code_cache: Option<Cow<'static, [u8]>>,
code_cache: Option<ModuleSourceCodeCache>,
) -> Self {
let module_url_found = if specifier == specifier_found {
None
Expand Down Expand Up @@ -422,7 +428,7 @@ impl ModuleSource {
code: &'static str,
specified: impl AsRef<str>,
found: impl AsRef<str>,
code_cache: Option<Cow<'static, [u8]>>,
code_cache: Option<ModuleSourceCodeCache>,
) -> Self {
let specified = specified.as_ref().to_string();
let found = found.as_ref().to_string();
Expand Down
34 changes: 24 additions & 10 deletions core/modules/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use crate::modules::ModuleError;
use crate::modules::ModuleInfo;
use crate::modules::ModuleRequest;
use crate::modules::ModuleSourceCode;
use crate::modules::ModuleSourceCodeCache;
use crate::modules::RequestedModuleType;
use crate::resolve_import;
use crate::resolve_url;
Expand Down Expand Up @@ -213,15 +214,27 @@ impl Future for DelayedSourceCodeFuture {
return Poll::Pending;
}
match mock_source_code(&inner.url) {
Some(src) => Poll::Ready(Ok(ModuleSource::for_test_with_redirect(
src.0,
inner.url.as_str(),
src.1,
inner
.code_cache
.as_ref()
.map(|code_cache| Cow::Owned(code_cache.clone())),
))),
Some(src) => {
let hash = {
use std::hash::Hash;
use std::hash::Hasher;
let mut hasher = twox_hash::XxHash64::default();
src.1.hash(&mut hasher);
hasher.finish()
};
Poll::Ready(Ok(ModuleSource::for_test_with_redirect(
src.0,
inner.url.as_str(),
src.1,
Some(ModuleSourceCodeCache {
hash,
data: inner
.code_cache
.as_ref()
.map(|code_cache| Cow::Owned(code_cache.clone())),
}),
)))
}
None => Poll::Ready(Err(MockError::LoadErr.into())),
}
}
Expand Down Expand Up @@ -275,7 +288,8 @@ impl ModuleLoader for MockLoader {

fn code_cache_ready(
&self,
module_specifier: &ModuleSpecifier,
module_specifier: ModuleSpecifier,
_hash: u64,
code_cache: &[u8],
) -> Pin<Box<dyn Future<Output = ()>>> {
let mut updated_code_cache = self.updated_code_cache.lock();
Expand Down
1 change: 0 additions & 1 deletion core/runtime/jsruntime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -892,7 +892,6 @@ impl JsRuntime {
loader,
exception_state.clone(),
import_meta_resolve_cb,
options.enable_code_cache,
));

if let Some((snapshotted_data, mut data_store)) = snapshotted_data {
Expand Down

0 comments on commit f0a7357

Please sign in to comment.