Skip to content

Commit

Permalink
fix(node): seperate worker module cache (#23634)
Browse files Browse the repository at this point in the history
Construct a new module graph container for workers instead of sharing it
with the main worker.

Fixes #17248
Fixes #23461

---------

Co-authored-by: David Sherret <dsherret@gmail.com>
  • Loading branch information
2 people authored and bartlomieju committed May 16, 2024
1 parent 925be25 commit 4137a08
Show file tree
Hide file tree
Showing 17 changed files with 503 additions and 401 deletions.
9 changes: 9 additions & 0 deletions cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use self::package_json::PackageJsonDeps;
use ::import_map::ImportMap;
use deno_ast::SourceMapOption;
use deno_core::resolve_url_or_path;
use deno_graph::GraphKind;
use deno_npm::resolution::ValidSerializedNpmResolutionSnapshot;
use deno_npm::NpmSystemInfo;
use deno_runtime::deno_tls::RootCertStoreProvider;
Expand Down Expand Up @@ -873,6 +874,14 @@ impl CliOptions {
self.maybe_config_file.as_ref().map(|f| f.specifier.clone())
}

pub fn graph_kind(&self) -> GraphKind {
match self.sub_command() {
DenoSubcommand::Cache(_) => GraphKind::All,
DenoSubcommand::Check(_) => GraphKind::TypesOnly,
_ => self.type_check_mode().as_graph_kind(),
}
}

pub fn ts_type_lib_window(&self) -> TsTypeLib {
TsTypeLib::DenoWindow
}
Expand Down
48 changes: 24 additions & 24 deletions cli/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ use crate::cache::NodeAnalysisCache;
use crate::cache::ParsedSourceCache;
use crate::emit::Emitter;
use crate::file_fetcher::FileFetcher;
use crate::graph_container::MainModuleGraphContainer;
use crate::graph_util::FileWatcherReporter;
use crate::graph_util::ModuleGraphBuilder;
use crate::graph_util::ModuleGraphContainer;
use crate::graph_util::ModuleGraphCreator;
use crate::http_util::HttpClient;
use crate::module_loader::CliModuleLoaderFactory;
Expand Down Expand Up @@ -60,7 +60,6 @@ use deno_core::futures::FutureExt;
use deno_core::parking_lot::Mutex;
use deno_core::FeatureChecker;

use deno_graph::GraphKind;
use deno_lockfile::WorkspaceMemberConfig;
use deno_runtime::deno_fs;
use deno_runtime::deno_node::analyze::NodeCodeTranslator;
Expand Down Expand Up @@ -157,7 +156,7 @@ struct CliFactoryServices {
emit_cache: Deferred<EmitCache>,
emitter: Deferred<Arc<Emitter>>,
fs: Deferred<Arc<dyn deno_fs::FileSystem>>,
graph_container: Deferred<Arc<ModuleGraphContainer>>,
main_graph_container: Deferred<Arc<MainModuleGraphContainer>>,
lockfile: Deferred<Option<Arc<Mutex<Lockfile>>>>,
maybe_import_map: Deferred<Option<Arc<ImportMap>>>,
maybe_inspector_server: Deferred<Option<Arc<InspectorServer>>>,
Expand Down Expand Up @@ -673,17 +672,19 @@ impl CliFactory {
.await
}

pub fn graph_container(&self) -> &Arc<ModuleGraphContainer> {
self.services.graph_container.get_or_init(|| {
let graph_kind = match self.options.sub_command() {
// todo(dsherret): ideally the graph container would not be used
// for deno cache because it doesn't dynamically load modules
DenoSubcommand::Cache(_) => GraphKind::All,
DenoSubcommand::Check(_) => GraphKind::TypesOnly,
_ => self.options.type_check_mode().as_graph_kind(),
};
Arc::new(ModuleGraphContainer::new(graph_kind))
})
pub async fn main_module_graph_container(
&self,
) -> Result<&Arc<MainModuleGraphContainer>, AnyError> {
self
.services
.main_graph_container
.get_or_try_init_async(async {
Ok(Arc::new(MainModuleGraphContainer::new(
self.cli_options().clone(),
self.module_load_preparer().await?.clone(),
)))
})
.await
}

pub fn maybe_inspector_server(
Expand All @@ -706,7 +707,6 @@ impl CliFactory {
.get_or_try_init_async(async {
Ok(Arc::new(ModuleLoadPreparer::new(
self.options.clone(),
self.graph_container().clone(),
self.maybe_lockfile().clone(),
self.module_graph_builder().await?.clone(),
self.text_only_progress_bar().clone(),
Expand Down Expand Up @@ -791,24 +791,24 @@ impl CliFactory {
self.blob_store().clone(),
Box::new(CliModuleLoaderFactory::new(
&self.options,
if self.options.code_cache_enabled() {
Some(self.code_cache()?.clone())
} else {
None
},
self.emitter()?.clone(),
self.graph_container().clone(),
self.main_module_graph_container().await?.clone(),
self.module_info_cache()?.clone(),
self.module_load_preparer().await?.clone(),
self.parsed_source_cache().clone(),
self.resolver().await?.clone(),
cli_node_resolver.clone(),
NpmModuleLoader::new(
self.cjs_resolutions().clone(),
self.node_code_translator().await?.clone(),
fs.clone(),
cli_node_resolver.clone(),
),
if self.options.code_cache_enabled() {
Some(self.code_cache()?.clone())
} else {
None
},
self.module_info_cache()?.clone(),
self.parsed_source_cache().clone(),
self.resolver().await?.clone(),
)),
self.root_cert_store_provider().clone(),
self.fs().clone(),
Expand Down
157 changes: 157 additions & 0 deletions cli/graph_container.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,157 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.

use std::sync::Arc;

use deno_ast::ModuleSpecifier;
use deno_core::error::AnyError;
use deno_core::parking_lot::RwLock;
use deno_core::resolve_url_or_path;
use deno_graph::ModuleGraph;
use deno_runtime::colors;
use deno_runtime::permissions::PermissionsContainer;

use crate::args::CliOptions;
use crate::module_loader::ModuleLoadPreparer;

pub trait ModuleGraphContainer: Clone + 'static {
/// Acquires a permit to modify the module graph without other code
/// having the chance to modify it. In the meantime, other code may
/// still read from the existing module graph.
async fn acquire_update_permit(&self) -> impl ModuleGraphUpdatePermit;
/// Gets a copy of the graph.
fn graph(&self) -> Arc<ModuleGraph>;
}

/// A permit for updating the module graph. When complete and
/// everything looks fine, calling `.commit()` will store the
/// new graph in the ModuleGraphContainer.
pub trait ModuleGraphUpdatePermit {
/// Gets the module graph for mutation.
fn graph_mut(&mut self) -> &mut ModuleGraph;
/// Saves the mutated module graph in the container.
fn commit(self);
}

/// Holds the `ModuleGraph` for the main worker.
#[derive(Clone)]
pub struct MainModuleGraphContainer {
// Allow only one request to update the graph data at a time,
// but allow other requests to read from it at any time even
// while another request is updating the data.
update_queue: Arc<crate::util::sync::TaskQueue>,
inner: Arc<RwLock<Arc<ModuleGraph>>>,
cli_options: Arc<CliOptions>,
module_load_preparer: Arc<ModuleLoadPreparer>,
}

impl MainModuleGraphContainer {
pub fn new(
cli_options: Arc<CliOptions>,
module_load_preparer: Arc<ModuleLoadPreparer>,
) -> Self {
Self {
update_queue: Default::default(),
inner: Arc::new(RwLock::new(Arc::new(ModuleGraph::new(
cli_options.graph_kind(),
)))),
cli_options,
module_load_preparer,
}
}

pub async fn check_specifiers(
&self,
specifiers: &[ModuleSpecifier],
) -> Result<(), AnyError> {
let mut graph_permit = self.acquire_update_permit().await;
let graph = graph_permit.graph_mut();
self
.module_load_preparer
.prepare_module_load(
graph,
specifiers,
false,
self.cli_options.ts_type_lib_window(),
PermissionsContainer::allow_all(),
)
.await?;
graph_permit.commit();
Ok(())
}

/// Helper around prepare_module_load that loads and type checks
/// the provided files.
pub async fn load_and_type_check_files(
&self,
files: &[String],
) -> Result<(), AnyError> {
let specifiers = self.collect_specifiers(files)?;

if specifiers.is_empty() {
log::warn!("{} No matching files found.", colors::yellow("Warning"));
}

self.check_specifiers(&specifiers).await
}

pub fn collect_specifiers(
&self,
files: &[String],
) -> Result<Vec<ModuleSpecifier>, AnyError> {
let excludes = self.cli_options.resolve_config_excludes()?;
Ok(
files
.iter()
.filter_map(|file| {
let file_url =
resolve_url_or_path(file, self.cli_options.initial_cwd()).ok()?;
if file_url.scheme() != "file" {
return Some(file_url);
}
// ignore local files that match any of files listed in `exclude` option
let file_path = file_url.to_file_path().ok()?;
if excludes.matches_path(&file_path) {
None
} else {
Some(file_url)
}
})
.collect::<Vec<_>>(),
)
}
}

impl ModuleGraphContainer for MainModuleGraphContainer {
async fn acquire_update_permit(&self) -> impl ModuleGraphUpdatePermit {
let permit = self.update_queue.acquire().await;
MainModuleGraphUpdatePermit {
permit,
inner: self.inner.clone(),
graph: (**self.inner.read()).clone(),
}
}

fn graph(&self) -> Arc<ModuleGraph> {
self.inner.read().clone()
}
}

/// A permit for updating the module graph. When complete and
/// everything looks fine, calling `.commit()` will store the
/// new graph in the ModuleGraphContainer.
pub struct MainModuleGraphUpdatePermit<'a> {
permit: crate::util::sync::TaskQueuePermit<'a>,
inner: Arc<RwLock<Arc<ModuleGraph>>>,
graph: ModuleGraph,
}

impl<'a> ModuleGraphUpdatePermit for MainModuleGraphUpdatePermit<'a> {
fn graph_mut(&mut self) -> &mut ModuleGraph {
&mut self.graph
}

fn commit(self) {
*self.inner.write() = Arc::new(self.graph);
drop(self.permit); // explicit drop for clarity
}
}
62 changes: 0 additions & 62 deletions cli/graph_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,13 @@ use crate::tools::check;
use crate::tools::check::TypeChecker;
use crate::util::file_watcher::WatcherCommunicator;
use crate::util::fs::canonicalize_path;
use crate::util::sync::TaskQueue;
use crate::util::sync::TaskQueuePermit;
use deno_runtime::fs_util::specifier_to_file_path;

use deno_config::WorkspaceMemberConfig;
use deno_core::anyhow::bail;
use deno_core::error::custom_error;
use deno_core::error::AnyError;
use deno_core::parking_lot::Mutex;
use deno_core::parking_lot::RwLock;
use deno_core::ModuleSpecifier;
use deno_graph::source::Loader;
use deno_graph::source::ResolutionMode;
Expand Down Expand Up @@ -762,40 +759,6 @@ fn get_resolution_error_bare_specifier(
}
}

/// Holds the `ModuleGraph` and what parts of it are type checked.
pub struct ModuleGraphContainer {
// Allow only one request to update the graph data at a time,
// but allow other requests to read from it at any time even
// while another request is updating the data.
update_queue: Arc<TaskQueue>,
inner: Arc<RwLock<Arc<ModuleGraph>>>,
}

impl ModuleGraphContainer {
pub fn new(graph_kind: GraphKind) -> Self {
Self {
update_queue: Default::default(),
inner: Arc::new(RwLock::new(Arc::new(ModuleGraph::new(graph_kind)))),
}
}

/// Acquires a permit to modify the module graph without other code
/// having the chance to modify it. In the meantime, other code may
/// still read from the existing module graph.
pub async fn acquire_update_permit(&self) -> ModuleGraphUpdatePermit {
let permit = self.update_queue.acquire().await;
ModuleGraphUpdatePermit {
permit,
inner: self.inner.clone(),
graph: (**self.inner.read()).clone(),
}
}

pub fn graph(&self) -> Arc<ModuleGraph> {
self.inner.read().clone()
}
}

/// Gets if any of the specified root's "file:" dependents are in the
/// provided changed set.
pub fn has_graph_root_local_dependent_changed(
Expand Down Expand Up @@ -828,31 +791,6 @@ pub fn has_graph_root_local_dependent_changed(
false
}

/// A permit for updating the module graph. When complete and
/// everything looks fine, calling `.commit()` will store the
/// new graph in the ModuleGraphContainer.
pub struct ModuleGraphUpdatePermit<'a> {
permit: TaskQueuePermit<'a>,
inner: Arc<RwLock<Arc<ModuleGraph>>>,
graph: ModuleGraph,
}

impl<'a> ModuleGraphUpdatePermit<'a> {
/// Gets the module graph for mutation.
pub fn graph_mut(&mut self) -> &mut ModuleGraph {
&mut self.graph
}

/// Saves the mutated module graph in the container
/// and returns an Arc to the new module graph.
pub fn commit(self) -> Arc<ModuleGraph> {
let graph = Arc::new(self.graph);
*self.inner.write() = graph.clone();
drop(self.permit); // explicit drop for clarity
graph
}
}

#[derive(Clone, Debug)]
pub struct FileWatcherReporter {
watcher_communicator: Arc<WatcherCommunicator>,
Expand Down
4 changes: 2 additions & 2 deletions cli/lsp/testing/execution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,10 @@ impl TestRun {
// file would have impact on other files, which is undesirable.
let permissions =
Permissions::from_options(&factory.cli_options().permissions_options()?)?;
let main_graph_container = factory.main_module_graph_container().await?;
test::check_specifiers(
factory.cli_options(),
factory.file_fetcher()?,
factory.module_load_preparer().await?,
main_graph_container,
self
.queue
.iter()
Expand Down

0 comments on commit 4137a08

Please sign in to comment.