Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: clear dep analysis when module loading is done #27204

Merged
merged 11 commits into from
Dec 5, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions Cargo.lock

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

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ repository = "https://github.com/denoland/deno"

[workspace.dependencies]
deno_ast = { version = "=0.44.0", features = ["transpiling"] }
deno_core = { version = "0.323.0" }
deno_core = { version = "0.324.0" }

deno_bench_util = { version = "0.174.0", path = "./bench_util" }
deno_config = { version = "=0.39.3", features = ["workspace", "sync"] }
Expand Down
10 changes: 10 additions & 0 deletions cli/cache/parsed_source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,21 @@ impl ParsedSourceCache {
self.sources.lock().remove(specifier);
}

/// Fress all parsed sources from memory.
pub fn free_all(&self) {
self.sources.lock().clear();
}

/// Creates a parser that will reuse a ParsedSource from the store
/// if it exists, or else parse.
pub fn as_capturing_parser(&self) -> CapturingEsParser {
CapturingEsParser::new(None, self)
}

#[cfg(test)]
pub fn len(&self) -> usize {
self.sources.lock().len()
}
}

/// It's ok that this is racy since in non-LSP situations
Expand Down
90 changes: 90 additions & 0 deletions cli/module_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ use std::path::PathBuf;
use std::pin::Pin;
use std::rc::Rc;
use std::str;
use std::sync::atomic::AtomicU16;
use std::sync::atomic::Ordering;
use std::sync::Arc;

use crate::args::jsr_url;
Expand Down Expand Up @@ -49,6 +51,7 @@ use deno_core::error::generic_error;
use deno_core::error::AnyError;
use deno_core::futures::future::FutureExt;
use deno_core::futures::Future;
use deno_core::parking_lot::Mutex;
use deno_core::resolve_url;
use deno_core::ModuleCodeString;
use deno_core::ModuleLoader;
Expand Down Expand Up @@ -222,6 +225,39 @@ struct SharedCliModuleLoaderState {
npm_module_loader: NpmModuleLoader,
parsed_source_cache: Arc<ParsedSourceCache>,
resolver: Arc<CliResolver>,
in_flight_loads_tracker: InFlightModuleLoadsTracker,
}

struct InFlightModuleLoadsTracker {
bartlomieju marked this conversation as resolved.
Show resolved Hide resolved
loads_number: Arc<AtomicU16>,
cleanup_task_timeout: u64,
cleanup_task_handle: Arc<Mutex<Option<tokio::task::JoinHandle<()>>>>,
}

impl InFlightModuleLoadsTracker {
pub fn increase(&self) {
self.loads_number.fetch_add(1, Ordering::Relaxed);
if let Some(task) = self.cleanup_task_handle.lock().take() {
task.abort();
}
}

pub fn decrease(&self, parsed_source_cache: &Arc<ParsedSourceCache>) {
let prev = self.loads_number.fetch_sub(1, Ordering::Relaxed);
if prev == 1 {
let parsed_source_cache = parsed_source_cache.clone();
let timeout = self.cleanup_task_timeout;
let task_handle = tokio::spawn(async move {
tokio::time::sleep(std::time::Duration::from_millis(timeout)).await;
bartlomieju marked this conversation as resolved.
Show resolved Hide resolved
parsed_source_cache.free_all();
});
let maybe_prev_task =
self.cleanup_task_handle.lock().replace(task_handle);
if let Some(prev_task) = maybe_prev_task {
prev_task.abort();
}
}
}
}

pub struct CliModuleLoaderFactory {
Expand Down Expand Up @@ -272,6 +308,11 @@ impl CliModuleLoaderFactory {
npm_module_loader,
parsed_source_cache,
resolver,
in_flight_loads_tracker: InFlightModuleLoadsTracker {
loads_number: Arc::new(AtomicU16::new(0)),
cleanup_task_timeout: 10_000,
cleanup_task_handle: Arc::new(Mutex::new(None)),
},
}),
}
}
Expand Down Expand Up @@ -867,6 +908,7 @@ impl<TGraphContainer: ModuleGraphContainer> ModuleLoader
_maybe_referrer: Option<String>,
is_dynamic: bool,
) -> Pin<Box<dyn Future<Output = Result<(), AnyError>>>> {
self.0.shared.in_flight_loads_tracker.increase();
if self.0.shared.in_npm_pkg_checker.in_npm_package(specifier) {
return Box::pin(deno_core::futures::future::ready(Ok(())));
}
Expand Down Expand Up @@ -921,6 +963,14 @@ impl<TGraphContainer: ModuleGraphContainer> ModuleLoader
.boxed_local()
}

fn finish_load(&self) {
self
.0
.shared
.in_flight_loads_tracker
.decrease(&self.0.shared.parsed_source_cache);
}

fn code_cache_ready(
&self,
specifier: ModuleSpecifier,
Expand Down Expand Up @@ -1103,3 +1153,43 @@ impl<TGraphContainer: ModuleGraphContainer> NodeRequireLoader
self.cjs_tracker.is_maybe_cjs(specifier, media_type)
}
}

#[cfg(test)]
mod tests {
use super::*;
use deno_graph::ParsedSourceStore;

#[tokio::test]
async fn test_inflight_module_loads_tracker() {
let tracker = InFlightModuleLoadsTracker {
loads_number: Default::default(),
cleanup_task_timeout: 10,
cleanup_task_handle: Default::default(),
};

let specifier = ModuleSpecifier::parse("file:///a.js").unwrap();
let source = "const a = 'hello';";
let parsed_source_cache = Arc::new(ParsedSourceCache::default());
let parsed_source = parsed_source_cache
.remove_or_parse_module(&specifier, source.into(), MediaType::JavaScript)
.unwrap();
parsed_source_cache.set_parsed_source(specifier, parsed_source);

assert_eq!(parsed_source_cache.len(), 1);
assert!(tracker.cleanup_task_handle.lock().is_none());
tracker.increase();
tracker.increase();
assert!(tracker.cleanup_task_handle.lock().is_none());
tracker.decrease(&parsed_source_cache);
assert!(tracker.cleanup_task_handle.lock().is_none());
tracker.decrease(&parsed_source_cache);
assert!(tracker.cleanup_task_handle.lock().is_some());
assert_eq!(parsed_source_cache.len(), 1);
tracker.increase();
assert!(tracker.cleanup_task_handle.lock().is_none());
assert_eq!(parsed_source_cache.len(), 1);
tracker.decrease(&parsed_source_cache);
tokio::time::sleep(std::time::Duration::from_millis(20)).await;
assert_eq!(parsed_source_cache.len(), 0);
}
}
Loading