Skip to content

Commit 4137a08

Browse files
littledivydsherret
authored andcommitted
fix(node): seperate worker module cache (#23634)
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 <[email protected]>
1 parent 925be25 commit 4137a08

File tree

17 files changed

+503
-401
lines changed

17 files changed

+503
-401
lines changed

cli/args/mod.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ use self::package_json::PackageJsonDeps;
1212
use ::import_map::ImportMap;
1313
use deno_ast::SourceMapOption;
1414
use deno_core::resolve_url_or_path;
15+
use deno_graph::GraphKind;
1516
use deno_npm::resolution::ValidSerializedNpmResolutionSnapshot;
1617
use deno_npm::NpmSystemInfo;
1718
use deno_runtime::deno_tls::RootCertStoreProvider;
@@ -873,6 +874,14 @@ impl CliOptions {
873874
self.maybe_config_file.as_ref().map(|f| f.specifier.clone())
874875
}
875876

877+
pub fn graph_kind(&self) -> GraphKind {
878+
match self.sub_command() {
879+
DenoSubcommand::Cache(_) => GraphKind::All,
880+
DenoSubcommand::Check(_) => GraphKind::TypesOnly,
881+
_ => self.type_check_mode().as_graph_kind(),
882+
}
883+
}
884+
876885
pub fn ts_type_lib_window(&self) -> TsTypeLib {
877886
TsTypeLib::DenoWindow
878887
}

cli/factory.rs

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,9 @@ use crate::cache::NodeAnalysisCache;
2121
use crate::cache::ParsedSourceCache;
2222
use crate::emit::Emitter;
2323
use crate::file_fetcher::FileFetcher;
24+
use crate::graph_container::MainModuleGraphContainer;
2425
use crate::graph_util::FileWatcherReporter;
2526
use crate::graph_util::ModuleGraphBuilder;
26-
use crate::graph_util::ModuleGraphContainer;
2727
use crate::graph_util::ModuleGraphCreator;
2828
use crate::http_util::HttpClient;
2929
use crate::module_loader::CliModuleLoaderFactory;
@@ -60,7 +60,6 @@ use deno_core::futures::FutureExt;
6060
use deno_core::parking_lot::Mutex;
6161
use deno_core::FeatureChecker;
6262

63-
use deno_graph::GraphKind;
6463
use deno_lockfile::WorkspaceMemberConfig;
6564
use deno_runtime::deno_fs;
6665
use deno_runtime::deno_node::analyze::NodeCodeTranslator;
@@ -157,7 +156,7 @@ struct CliFactoryServices {
157156
emit_cache: Deferred<EmitCache>,
158157
emitter: Deferred<Arc<Emitter>>,
159158
fs: Deferred<Arc<dyn deno_fs::FileSystem>>,
160-
graph_container: Deferred<Arc<ModuleGraphContainer>>,
159+
main_graph_container: Deferred<Arc<MainModuleGraphContainer>>,
161160
lockfile: Deferred<Option<Arc<Mutex<Lockfile>>>>,
162161
maybe_import_map: Deferred<Option<Arc<ImportMap>>>,
163162
maybe_inspector_server: Deferred<Option<Arc<InspectorServer>>>,
@@ -673,17 +672,19 @@ impl CliFactory {
673672
.await
674673
}
675674

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

689690
pub fn maybe_inspector_server(
@@ -706,7 +707,6 @@ impl CliFactory {
706707
.get_or_try_init_async(async {
707708
Ok(Arc::new(ModuleLoadPreparer::new(
708709
self.options.clone(),
709-
self.graph_container().clone(),
710710
self.maybe_lockfile().clone(),
711711
self.module_graph_builder().await?.clone(),
712712
self.text_only_progress_bar().clone(),
@@ -791,24 +791,24 @@ impl CliFactory {
791791
self.blob_store().clone(),
792792
Box::new(CliModuleLoaderFactory::new(
793793
&self.options,
794+
if self.options.code_cache_enabled() {
795+
Some(self.code_cache()?.clone())
796+
} else {
797+
None
798+
},
794799
self.emitter()?.clone(),
795-
self.graph_container().clone(),
800+
self.main_module_graph_container().await?.clone(),
801+
self.module_info_cache()?.clone(),
796802
self.module_load_preparer().await?.clone(),
797-
self.parsed_source_cache().clone(),
798-
self.resolver().await?.clone(),
799803
cli_node_resolver.clone(),
800804
NpmModuleLoader::new(
801805
self.cjs_resolutions().clone(),
802806
self.node_code_translator().await?.clone(),
803807
fs.clone(),
804808
cli_node_resolver.clone(),
805809
),
806-
if self.options.code_cache_enabled() {
807-
Some(self.code_cache()?.clone())
808-
} else {
809-
None
810-
},
811-
self.module_info_cache()?.clone(),
810+
self.parsed_source_cache().clone(),
811+
self.resolver().await?.clone(),
812812
)),
813813
self.root_cert_store_provider().clone(),
814814
self.fs().clone(),

cli/graph_container.rs

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.
2+
3+
use std::sync::Arc;
4+
5+
use deno_ast::ModuleSpecifier;
6+
use deno_core::error::AnyError;
7+
use deno_core::parking_lot::RwLock;
8+
use deno_core::resolve_url_or_path;
9+
use deno_graph::ModuleGraph;
10+
use deno_runtime::colors;
11+
use deno_runtime::permissions::PermissionsContainer;
12+
13+
use crate::args::CliOptions;
14+
use crate::module_loader::ModuleLoadPreparer;
15+
16+
pub trait ModuleGraphContainer: Clone + 'static {
17+
/// Acquires a permit to modify the module graph without other code
18+
/// having the chance to modify it. In the meantime, other code may
19+
/// still read from the existing module graph.
20+
async fn acquire_update_permit(&self) -> impl ModuleGraphUpdatePermit;
21+
/// Gets a copy of the graph.
22+
fn graph(&self) -> Arc<ModuleGraph>;
23+
}
24+
25+
/// A permit for updating the module graph. When complete and
26+
/// everything looks fine, calling `.commit()` will store the
27+
/// new graph in the ModuleGraphContainer.
28+
pub trait ModuleGraphUpdatePermit {
29+
/// Gets the module graph for mutation.
30+
fn graph_mut(&mut self) -> &mut ModuleGraph;
31+
/// Saves the mutated module graph in the container.
32+
fn commit(self);
33+
}
34+
35+
/// Holds the `ModuleGraph` for the main worker.
36+
#[derive(Clone)]
37+
pub struct MainModuleGraphContainer {
38+
// Allow only one request to update the graph data at a time,
39+
// but allow other requests to read from it at any time even
40+
// while another request is updating the data.
41+
update_queue: Arc<crate::util::sync::TaskQueue>,
42+
inner: Arc<RwLock<Arc<ModuleGraph>>>,
43+
cli_options: Arc<CliOptions>,
44+
module_load_preparer: Arc<ModuleLoadPreparer>,
45+
}
46+
47+
impl MainModuleGraphContainer {
48+
pub fn new(
49+
cli_options: Arc<CliOptions>,
50+
module_load_preparer: Arc<ModuleLoadPreparer>,
51+
) -> Self {
52+
Self {
53+
update_queue: Default::default(),
54+
inner: Arc::new(RwLock::new(Arc::new(ModuleGraph::new(
55+
cli_options.graph_kind(),
56+
)))),
57+
cli_options,
58+
module_load_preparer,
59+
}
60+
}
61+
62+
pub async fn check_specifiers(
63+
&self,
64+
specifiers: &[ModuleSpecifier],
65+
) -> Result<(), AnyError> {
66+
let mut graph_permit = self.acquire_update_permit().await;
67+
let graph = graph_permit.graph_mut();
68+
self
69+
.module_load_preparer
70+
.prepare_module_load(
71+
graph,
72+
specifiers,
73+
false,
74+
self.cli_options.ts_type_lib_window(),
75+
PermissionsContainer::allow_all(),
76+
)
77+
.await?;
78+
graph_permit.commit();
79+
Ok(())
80+
}
81+
82+
/// Helper around prepare_module_load that loads and type checks
83+
/// the provided files.
84+
pub async fn load_and_type_check_files(
85+
&self,
86+
files: &[String],
87+
) -> Result<(), AnyError> {
88+
let specifiers = self.collect_specifiers(files)?;
89+
90+
if specifiers.is_empty() {
91+
log::warn!("{} No matching files found.", colors::yellow("Warning"));
92+
}
93+
94+
self.check_specifiers(&specifiers).await
95+
}
96+
97+
pub fn collect_specifiers(
98+
&self,
99+
files: &[String],
100+
) -> Result<Vec<ModuleSpecifier>, AnyError> {
101+
let excludes = self.cli_options.resolve_config_excludes()?;
102+
Ok(
103+
files
104+
.iter()
105+
.filter_map(|file| {
106+
let file_url =
107+
resolve_url_or_path(file, self.cli_options.initial_cwd()).ok()?;
108+
if file_url.scheme() != "file" {
109+
return Some(file_url);
110+
}
111+
// ignore local files that match any of files listed in `exclude` option
112+
let file_path = file_url.to_file_path().ok()?;
113+
if excludes.matches_path(&file_path) {
114+
None
115+
} else {
116+
Some(file_url)
117+
}
118+
})
119+
.collect::<Vec<_>>(),
120+
)
121+
}
122+
}
123+
124+
impl ModuleGraphContainer for MainModuleGraphContainer {
125+
async fn acquire_update_permit(&self) -> impl ModuleGraphUpdatePermit {
126+
let permit = self.update_queue.acquire().await;
127+
MainModuleGraphUpdatePermit {
128+
permit,
129+
inner: self.inner.clone(),
130+
graph: (**self.inner.read()).clone(),
131+
}
132+
}
133+
134+
fn graph(&self) -> Arc<ModuleGraph> {
135+
self.inner.read().clone()
136+
}
137+
}
138+
139+
/// A permit for updating the module graph. When complete and
140+
/// everything looks fine, calling `.commit()` will store the
141+
/// new graph in the ModuleGraphContainer.
142+
pub struct MainModuleGraphUpdatePermit<'a> {
143+
permit: crate::util::sync::TaskQueuePermit<'a>,
144+
inner: Arc<RwLock<Arc<ModuleGraph>>>,
145+
graph: ModuleGraph,
146+
}
147+
148+
impl<'a> ModuleGraphUpdatePermit for MainModuleGraphUpdatePermit<'a> {
149+
fn graph_mut(&mut self) -> &mut ModuleGraph {
150+
&mut self.graph
151+
}
152+
153+
fn commit(self) {
154+
*self.inner.write() = Arc::new(self.graph);
155+
drop(self.permit); // explicit drop for clarity
156+
}
157+
}

cli/graph_util.rs

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,13 @@ use crate::tools::check;
1818
use crate::tools::check::TypeChecker;
1919
use crate::util::file_watcher::WatcherCommunicator;
2020
use crate::util::fs::canonicalize_path;
21-
use crate::util::sync::TaskQueue;
22-
use crate::util::sync::TaskQueuePermit;
2321
use deno_runtime::fs_util::specifier_to_file_path;
2422

2523
use deno_config::WorkspaceMemberConfig;
2624
use deno_core::anyhow::bail;
2725
use deno_core::error::custom_error;
2826
use deno_core::error::AnyError;
2927
use deno_core::parking_lot::Mutex;
30-
use deno_core::parking_lot::RwLock;
3128
use deno_core::ModuleSpecifier;
3229
use deno_graph::source::Loader;
3330
use deno_graph::source::ResolutionMode;
@@ -762,40 +759,6 @@ fn get_resolution_error_bare_specifier(
762759
}
763760
}
764761

765-
/// Holds the `ModuleGraph` and what parts of it are type checked.
766-
pub struct ModuleGraphContainer {
767-
// Allow only one request to update the graph data at a time,
768-
// but allow other requests to read from it at any time even
769-
// while another request is updating the data.
770-
update_queue: Arc<TaskQueue>,
771-
inner: Arc<RwLock<Arc<ModuleGraph>>>,
772-
}
773-
774-
impl ModuleGraphContainer {
775-
pub fn new(graph_kind: GraphKind) -> Self {
776-
Self {
777-
update_queue: Default::default(),
778-
inner: Arc::new(RwLock::new(Arc::new(ModuleGraph::new(graph_kind)))),
779-
}
780-
}
781-
782-
/// Acquires a permit to modify the module graph without other code
783-
/// having the chance to modify it. In the meantime, other code may
784-
/// still read from the existing module graph.
785-
pub async fn acquire_update_permit(&self) -> ModuleGraphUpdatePermit {
786-
let permit = self.update_queue.acquire().await;
787-
ModuleGraphUpdatePermit {
788-
permit,
789-
inner: self.inner.clone(),
790-
graph: (**self.inner.read()).clone(),
791-
}
792-
}
793-
794-
pub fn graph(&self) -> Arc<ModuleGraph> {
795-
self.inner.read().clone()
796-
}
797-
}
798-
799762
/// Gets if any of the specified root's "file:" dependents are in the
800763
/// provided changed set.
801764
pub fn has_graph_root_local_dependent_changed(
@@ -828,31 +791,6 @@ pub fn has_graph_root_local_dependent_changed(
828791
false
829792
}
830793

831-
/// A permit for updating the module graph. When complete and
832-
/// everything looks fine, calling `.commit()` will store the
833-
/// new graph in the ModuleGraphContainer.
834-
pub struct ModuleGraphUpdatePermit<'a> {
835-
permit: TaskQueuePermit<'a>,
836-
inner: Arc<RwLock<Arc<ModuleGraph>>>,
837-
graph: ModuleGraph,
838-
}
839-
840-
impl<'a> ModuleGraphUpdatePermit<'a> {
841-
/// Gets the module graph for mutation.
842-
pub fn graph_mut(&mut self) -> &mut ModuleGraph {
843-
&mut self.graph
844-
}
845-
846-
/// Saves the mutated module graph in the container
847-
/// and returns an Arc to the new module graph.
848-
pub fn commit(self) -> Arc<ModuleGraph> {
849-
let graph = Arc::new(self.graph);
850-
*self.inner.write() = graph.clone();
851-
drop(self.permit); // explicit drop for clarity
852-
graph
853-
}
854-
}
855-
856794
#[derive(Clone, Debug)]
857795
pub struct FileWatcherReporter {
858796
watcher_communicator: Arc<WatcherCommunicator>,

cli/lsp/testing/execution.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -219,10 +219,10 @@ impl TestRun {
219219
// file would have impact on other files, which is undesirable.
220220
let permissions =
221221
Permissions::from_options(&factory.cli_options().permissions_options()?)?;
222+
let main_graph_container = factory.main_module_graph_container().await?;
222223
test::check_specifiers(
223-
factory.cli_options(),
224224
factory.file_fetcher()?,
225-
factory.module_load_preparer().await?,
225+
main_graph_container,
226226
self
227227
.queue
228228
.iter()

0 commit comments

Comments
 (0)