foster/denoland-deno
1
0
Fork 0
mirror of https://github.com/denoland/deno.git synced 2024-11-21 15:04:11 -05:00
Code Issues Packages 1 Wiki Activity

fix(core/modules): Prepare modules only once per runtime (#11015)

Browse source 
This commit changes module loading implementation in "deno_core"
to call "ModuleLoader::prepare" hook only once per entry point.

This is done to avoid multiple type checking of the same code
in case of duplicated dynamic imports.

Relevant code in "cli/module_graph.rs" was updated as well.
This commit is contained in:
Nayeem Rahman 2021-06-19 15:14:43 +01:00 committed by GitHub
parent b0c04a7941
commit 2ea41d3ac1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 164 additions and 112 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

1
cli/main.rs
View file

@ -598,6 +598,7 @@ async fn create_module_graph_and_maybe_check(
lib,
maybe_config_file: program_state.maybe_config_file.clone(),
reload: program_state.flags.reload,
..Default::default()
})?;
debug!("{}", result_info.stats);

39
cli/module_graph.rs
View file

@ -624,6 +624,10 @@ pub struct CheckOptions {
/// Ignore any previously emits and ensure that all files are emitted from
/// source.
pub reload: bool,
/// A set of module specifiers to be excluded from the effect of
/// `CheckOptions::reload` if it is `true`. Perhaps because they have already
/// reloaded once in this process.
pub reload_exclusions: HashSet<ModuleSpecifier>,
}
#[derive(Debug, Eq, PartialEq)]
@ -673,6 +677,10 @@ pub struct TranspileOptions {
/// Ignore any previously emits and ensure that all files are emitted from
/// source.
pub reload: bool,
/// A set of module specifiers to be excluded from the effect of
/// `CheckOptions::reload` if it is `true`. Perhaps because they have already
/// reloaded once in this process.
pub reload_exclusions: HashSet<ModuleSpecifier>,
}
#[derive(Debug, Clone)]
@ -851,15 +859,14 @@ impl Graph {
let maybe_ignored_options = config
.merge_tsconfig_from_config_file(options.maybe_config_file.as_ref())?;
let needs_reload = options.reload
&& !self
.roots
.iter()
.all(|u| options.reload_exclusions.contains(u));
// Short circuit if none of the modules require an emit, or all of the
// modules that require an emit have a valid emit. There is also an edge
// case where there are multiple imports of a dynamic module during a
// single invocation, if that is the case, even if there is a reload, we
// will simply look at if the emit is invalid, to avoid two checks for the
// same programme.
if !self.needs_emit(&config)
|| (self.is_emit_valid(&config)
&& (!options.reload || self.roots_dynamic))
// modules that require an emit have a valid emit.
if !self.needs_emit(&config) || self.is_emit_valid(&config) && !needs_reload
{
debug!("graph does not need to be checked or emitted.");
return Ok(ResultInfo {
@ -1673,7 +1680,7 @@ impl Graph {
let check_js = ts_config.get_check_js();
let emit_options: ast::EmitOptions = ts_config.into();
let mut emit_count = 0_u32;
for (_, module_slot) in self.modules.iter_mut() {
for (specifier, module_slot) in self.modules.iter_mut() {
if let ModuleSlot::Module(module) = module_slot {
// TODO(kitsonk) a lot of this logic should be refactored into `Module` as
// we start to support other methods on the graph. Especially managing
@ -1692,8 +1699,11 @@ impl Graph {
{
continue;
}
let needs_reload =
options.reload && !options.reload_exclusions.contains(specifier);
// skip modules that already have a valid emit
if !options.reload && module.is_emit_valid(&config) {
if module.is_emit_valid(&config) && !needs_reload {
continue;
}
let parsed_module = module.parse()?;
@ -2255,6 +2265,7 @@ pub mod tests {
lib: TypeLib::DenoWindow,
maybe_config_file: None,
reload: false,
..Default::default()
})
.expect("should have checked");
assert!(result_info.maybe_ignored_options.is_none());
@ -2277,6 +2288,7 @@ pub mod tests {
lib: TypeLib::DenoWindow,
maybe_config_file: None,
reload: false,
..Default::default()
})
.expect("should have checked");
assert!(result_info.diagnostics.is_empty());
@ -2294,6 +2306,7 @@ pub mod tests {
lib: TypeLib::DenoWindow,
maybe_config_file: None,
reload: false,
..Default::default()
})
.expect("should have checked");
assert!(result_info.maybe_ignored_options.is_none());
@ -2318,6 +2331,7 @@ pub mod tests {
lib: TypeLib::DenoWindow,
maybe_config_file: None,
reload: false,
..Default::default()
})
.expect("should have checked");
assert!(result_info.maybe_ignored_options.is_none());
@ -2340,6 +2354,7 @@ pub mod tests {
lib: TypeLib::DenoWindow,
maybe_config_file: None,
reload: false,
..Default::default()
})
.expect("should have checked");
assert!(result_info.maybe_ignored_options.is_none());
@ -2361,6 +2376,7 @@ pub mod tests {
lib: TypeLib::DenoWindow,
maybe_config_file: None,
reload: false,
..Default::default()
})
.expect("should have checked");
assert!(result_info.diagnostics.is_empty());
@ -2380,6 +2396,7 @@ pub mod tests {
lib: TypeLib::DenoWindow,
maybe_config_file: Some(config_file),
reload: true,
..Default::default()
})
.expect("should have checked");
assert!(result_info.maybe_ignored_options.is_none());
@ -2401,6 +2418,7 @@ pub mod tests {
lib: TypeLib::DenoWindow,
maybe_config_file: Some(config_file),
reload: true,
..Default::default()
})
.expect("should have checked");
assert!(result_info.maybe_ignored_options.is_none());
@ -2628,6 +2646,7 @@ pub mod tests {
debug: false,
maybe_config_file: Some(config_file),
reload: false,
..Default::default()
})
.unwrap();
assert_eq!(

13
cli/program_state.rs
View file

@ -31,6 +31,7 @@ use deno_core::ModuleSpecifier;
use log::debug;
use log::warn;
use std::collections::HashMap;
use std::collections::HashSet;
use std::env;
use std::fs::read;
use std::sync::Arc;
@ -177,12 +178,17 @@ impl ProgramState {
let mut graph = builder.get_graph();
let debug = self.flags.log_level == Some(log::Level::Debug);
let maybe_config_file = self.maybe_config_file.clone();
let reload_exclusions = {
let modules = self.modules.lock().unwrap();
modules.keys().cloned().collect::<HashSet<_>>()
};
let result_modules = if self.flags.no_check {
let result_info = graph.transpile(TranspileOptions {
debug,
maybe_config_file,
reload: self.flags.reload,
reload_exclusions,
})?;
debug!("{}", result_info.stats);
if let Some(ignored_options) = result_info.maybe_ignored_options {
@ -196,6 +202,7 @@ impl ProgramState {
lib,
maybe_config_file,
reload: self.flags.reload,
reload_exclusions,
})?;
debug!("{}", result_info.stats);
@ -244,12 +251,17 @@ impl ProgramState {
let mut graph = builder.get_graph();
let debug = self.flags.log_level == Some(log::Level::Debug);
let maybe_config_file = self.maybe_config_file.clone();
let reload_exclusions = {
let modules = self.modules.lock().unwrap();
modules.keys().cloned().collect::<HashSet<_>>()
};
let result_modules = if self.flags.no_check {
let result_info = graph.transpile(TranspileOptions {
debug,
maybe_config_file,
reload: self.flags.reload,
reload_exclusions,
})?;
debug!("{}", result_info.stats);
if let Some(ignored_options) = result_info.maybe_ignored_options {
@ -263,6 +275,7 @@ impl ProgramState {
lib,
maybe_config_file,
reload: self.flags.reload,
reload_exclusions,
})?;
debug!("{}", result_info.stats);

14
cli/tests/single_compile_with_reload.ts
View file

@ -2,3 +2,17 @@ await import("./single_compile_with_reload_dyn.ts");
console.log("1");
await import("./single_compile_with_reload_dyn.ts");
console.log("2");
await new Promise((r) =>
new Worker(
new URL("single_compile_with_reload_worker.ts", import.meta.url).href,
{ type: "module" },
).onmessage = r
);
console.log("3");
await new Promise((r) =>
new Worker(
new URL("single_compile_with_reload_worker.ts", import.meta.url).href,
{ type: "module" },
).onmessage = r
);
console.log("4");

5
cli/tests/single_compile_with_reload.ts.out
View file

@ -2,3 +2,8 @@ Check [WILDCARD]single_compile_with_reload.ts
Hello
1
2
Check [WILDCARD]single_compile_with_reload_worker.ts
Hello from worker
3
Hello from worker
4

3
cli/tests/single_compile_with_reload_worker.ts Normal file
View file

@ -0,0 +1,3 @@
console.log("Hello from worker");
postMessage(null);
close();

189
core/modules.rs
View file

@ -180,16 +180,21 @@ impl ModuleLoader for FsModuleLoader {
}
}
#[derive(Debug, Eq, PartialEq)]
enum Kind {
Main,
DynamicImport,
/// Describes the entrypoint of a recursive module load.
#[derive(Debug)]
enum LoadInit {
/// Main module specifier.
Main(String),
/// Main module specifier with synthetic code for that module which bypasses
/// the loader.
MainWithCode(String, String),
/// Dynamic import specifier with referrer.
DynamicImport(String, String),
}
#[derive(Debug, Eq, PartialEq)]
pub enum LoadState {
ResolveMain(String, Option<String>),
ResolveImport(String, String),
Init,
LoadingRoot,
LoadingImports,
Done,
@ -198,7 +203,7 @@ pub enum LoadState {
/// This future is used to implement parallel async module loading.
pub struct RecursiveModuleLoad {
op_state: Rc<RefCell<OpState>>,
kind: Kind,
init: LoadInit,
// TODO(bartlomieju): in future this value should
// be randomized
pub id: ModuleLoadId,
@ -217,9 +222,12 @@ impl RecursiveModuleLoad {
code: Option<String>,
loader: Rc<dyn ModuleLoader>,
) -> Self {
let kind = Kind::Main;
let state = LoadState::ResolveMain(specifier.to_owned(), code);
Self::new(op_state, kind, state, loader)
let init = if let Some(code) = code {
LoadInit::MainWithCode(specifier.to_string(), code)
} else {
LoadInit::Main(specifier.to_string())
};
Self::new(op_state, init, loader)
}
pub fn dynamic_import(
@ -228,63 +236,54 @@ impl RecursiveModuleLoad {
referrer: &str,
loader: Rc<dyn ModuleLoader>,
) -> Self {
let kind = Kind::DynamicImport;
let state =
LoadState::ResolveImport(specifier.to_owned(), referrer.to_owned());
Self::new(op_state, kind, state, loader)
let init =
LoadInit::DynamicImport(specifier.to_string(), referrer.to_string());
Self::new(op_state, init, loader)
}
pub fn is_dynamic_import(&self) -> bool {
self.kind != Kind::Main
matches!(self.init, LoadInit::DynamicImport(..))
}
fn new(
op_state: Rc<RefCell<OpState>>,
kind: Kind,
state: LoadState,
init: LoadInit,
loader: Rc<dyn ModuleLoader>,
) -> Self {
Self {
id: NEXT_LOAD_ID.fetch_add(1, Ordering::SeqCst),
root_module_id: None,
op_state,
kind,
state,
init,
state: LoadState::Init,
loader,
pending: FuturesUnordered::new(),
is_pending: HashSet::new(),
}
}
pub async fn prepare(self) -> (ModuleLoadId, Result<Self, AnyError>) {
let (module_specifier, maybe_referrer) = match self.state {
LoadState::ResolveMain(ref specifier, _) => {
pub async fn prepare(&self) -> Result<(), AnyError> {
let (module_specifier, maybe_referrer) = match self.init {
LoadInit::Main(ref specifier)
| LoadInit::MainWithCode(ref specifier, _) => {
let spec =
match self
self
.loader
.resolve(self.op_state.clone(), specifier, ".", true)
{
Ok(spec) => spec,
Err(e) => return (self.id, Err(e)),
};
.resolve(self.op_state.clone(), specifier, ".", true)?;
(spec, None)
}
LoadState::ResolveImport(ref specifier, ref referrer) => {
let spec = match self.loader.resolve(
LoadInit::DynamicImport(ref specifier, ref referrer) => {
let spec = self.loader.resolve(
self.op_state.clone(),
specifier,
referrer,
false,
) {
Ok(spec) => spec,
Err(e) => return (self.id, Err(e)),
};
)?;
(spec, Some(referrer.to_string()))
}
_ => unreachable!(),
};
let prepare_result = self
self
.loader
.prepare_load(
self.op_state.clone(),
@ -293,52 +292,7 @@ impl RecursiveModuleLoad {
maybe_referrer,
self.is_dynamic_import(),
)
.await;
match prepare_result {
Ok(()) => (self.id, Ok(self)),
Err(e) => (self.id, Err(e)),
}
}
fn add_root(&mut self) -> Result<(), AnyError> {
let module_specifier = match self.state {
LoadState::ResolveMain(ref specifier, _) => {
self
.loader
.resolve(self.op_state.clone(), specifier, ".", true)?
}
LoadState::ResolveImport(ref specifier, ref referrer) => self
.loader
.resolve(self.op_state.clone(), specifier, referrer, false)?,
_ => unreachable!(),
};
let load_fut = match &self.state {
LoadState::ResolveMain(_, Some(code)) => {
futures::future::ok(ModuleSource {
code: code.to_owned(),
module_url_specified: module_specifier.to_string(),
module_url_found: module_specifier.to_string(),
})
.boxed()
}
_ => self
.loader
.load(
self.op_state.clone(),
&module_specifier,
None,
self.is_dynamic_import(),
)
.boxed_local(),
};
self.pending.push(load_fut);
self.state = LoadState::LoadingRoot;
Ok(())
.await
}
pub fn is_currently_loading_main_module(&self) -> bool {
@ -390,10 +344,38 @@ impl Stream for RecursiveModuleLoad {
) -> Poll<Option<Self::Item>> {
let inner = self.get_mut();
match inner.state {
LoadState::ResolveMain(..) | LoadState::ResolveImport(..) => {
if let Err(e) = inner.add_root() {
return Poll::Ready(Some(Err(e)));
}
LoadState::Init => {
let resolve_result = match inner.init {
LoadInit::Main(ref specifier)
| LoadInit::MainWithCode(ref specifier, _) => {
inner
.loader
.resolve(inner.op_state.clone(), specifier, ".", true)
}
LoadInit::DynamicImport(ref specifier, ref referrer) => inner
.loader
.resolve(inner.op_state.clone(), specifier, referrer, false),
};
let module_specifier = match resolve_result {
Ok(url) => url,
Err(error) => return Poll::Ready(Some(Err(error))),
};
let load_fut = match inner.init {
LoadInit::MainWithCode(_, ref code) => {
futures::future::ok(ModuleSource {
code: code.clone(),
module_url_specified: module_specifier.to_string(),
module_url_found: module_specifier.to_string(),
})
.boxed()
}
LoadInit::Main(..) | LoadInit::DynamicImport(..) => inner
.loader
.load(inner.op_state.clone(), &module_specifier, None, false)
.boxed_local(),
};
inner.pending.push(load_fut);
inner.state = LoadState::LoadingRoot;
inner.try_poll_next_unpin(cx)
}
LoadState::LoadingRoot | LoadState::LoadingImports => {
@ -648,17 +630,19 @@ impl ModuleMap {
self.info.get(id)
}
pub fn load_main(
pub async fn load_main(
&self,
specifier: &str,
code: Option<String>,
) -> RecursiveModuleLoad {
RecursiveModuleLoad::main(
) -> Result<RecursiveModuleLoad, AnyError> {
let load = RecursiveModuleLoad::main(
self.op_state.clone(),
specifier,
code,
self.loader.clone(),
)
);
load.prepare().await?;
Ok(load)
}
// Initiate loading of a module graph imported using `import()`.
@ -675,7 +659,21 @@ impl ModuleMap {
self.loader.clone(),
);
self.dynamic_import_map.insert(load.id, resolver_handle);
let fut = load.prepare().boxed_local();
let resolve_result =
load
.loader
.resolve(load.op_state.clone(), specifier, referrer, false);
let fut = match resolve_result {
Ok(module_specifier) => {
if self.is_registered(&module_specifier) {
async move { (load.id, Ok(load)) }.boxed_local()
} else {
async move { (load.id, load.prepare().await.map(|()| load)) }
.boxed_local()
}
}
Err(error) => async move { (load.id, Err(error)) }.boxed_local(),
};
self.preparing_dynamic_imports.push(fut);
}
@ -1128,13 +1126,12 @@ mod tests {
)
.unwrap();
assert_eq!(count.load(Ordering::Relaxed), 0);
// We should get an error here.
let result = runtime.poll_event_loop(cx, false);
if let Poll::Ready(Ok(_)) = result {
unreachable!();
}
assert_eq!(count.load(Ordering::Relaxed), 2);
assert_eq!(count.load(Ordering::Relaxed), 3);
})
}
@ -1154,7 +1151,7 @@ mod tests {
_is_main: bool,
) -> Result<ModuleSpecifier, AnyError> {
let c = self.resolve_count.fetch_add(1, Ordering::Relaxed);
assert!(c < 4);
assert!(c < 5);
assert_eq!(specifier, "./b.js");
assert_eq!(referrer, "file:///dyn_import3.js");
let s = crate::resolve_import(specifier, referrer).unwrap();
@ -1231,13 +1228,13 @@ mod tests {
runtime.poll_event_loop(cx, false),
Poll::Ready(Ok(_))
));
assert_eq!(resolve_count.load(Ordering::Relaxed), 4);
assert_eq!(resolve_count.load(Ordering::Relaxed), 5);
assert_eq!(load_count.load(Ordering::Relaxed), 2);
assert!(matches!(
runtime.poll_event_loop(cx, false),
Poll::Ready(Ok(_))
));
assert_eq!(resolve_count.load(Ordering::Relaxed), 4);
assert_eq!(resolve_count.load(Ordering::Relaxed), 5);
assert_eq!(load_count.load(Ordering::Relaxed), 2);
})
}

12
core/runtime.rs
View file

@ -1253,11 +1253,10 @@ impl JsRuntime {
) -> Result<ModuleId, AnyError> {
let module_map_rc = Self::module_map(self.v8_isolate());
let load = module_map_rc.borrow().load_main(specifier.as_str(), code);
let (_load_id, prepare_result) = load.prepare().await;
let mut load = prepare_result?;
let mut load = module_map_rc
.borrow()
.load_main(specifier.as_str(), code)
.await?;
while let Some(info_result) = load.next().await {
let info = info_result?;
@ -1268,7 +1267,8 @@ impl JsRuntime {
}
let root_id = load.expect_finished();
self.instantiate_module(root_id).map(|_| root_id)
self.instantiate_module(root_id)?;
Ok(root_id)
}
fn poll_pending_ops(