mirror of
https://github.com/denoland/deno
synced 2024-10-05 23:59:24 +00:00
perf(lsp): use lockfile to reduce npm pkg resolution time (#23247)
This functionality was broken. The series of events was: 1. Load the npm resolution from the lockfile. 2. Discover only a subset of the specifiers in the documents. 3. Clear the npm snapshot. 4. Redo npm resolution with the new specifiers (~500ms). What this now does: 1. Load the npm resolution from the lockfile. 2. Discover only a subset of the specifiers in the documents and take into account the specifiers from the lockfile. 3. Do not redo resolution (~1ms).
This commit is contained in:
parent
61f1b8e8dc
commit
83f92474c5
|
@ -855,6 +855,7 @@ pub struct Documents {
|
||||||
/// settings.
|
/// settings.
|
||||||
resolver: Arc<CliGraphResolver>,
|
resolver: Arc<CliGraphResolver>,
|
||||||
jsr_resolver: Arc<JsrCacheResolver>,
|
jsr_resolver: Arc<JsrCacheResolver>,
|
||||||
|
lockfile: Option<Arc<Mutex<Lockfile>>>,
|
||||||
/// The npm package requirements found in npm specifiers.
|
/// The npm package requirements found in npm specifiers.
|
||||||
npm_specifier_reqs: Arc<Vec<PackageReq>>,
|
npm_specifier_reqs: Arc<Vec<PackageReq>>,
|
||||||
/// Gets if any document had a node: specifier such that a @types/node package
|
/// Gets if any document had a node: specifier such that a @types/node package
|
||||||
|
@ -887,6 +888,7 @@ impl Documents {
|
||||||
sloppy_imports_resolver: None,
|
sloppy_imports_resolver: None,
|
||||||
})),
|
})),
|
||||||
jsr_resolver: Arc::new(JsrCacheResolver::new(cache.clone(), None)),
|
jsr_resolver: Arc::new(JsrCacheResolver::new(cache.clone(), None)),
|
||||||
|
lockfile: None,
|
||||||
npm_specifier_reqs: Default::default(),
|
npm_specifier_reqs: Default::default(),
|
||||||
has_injected_types_node_package: false,
|
has_injected_types_node_package: false,
|
||||||
redirect_resolver: Arc::new(RedirectResolver::new(cache)),
|
redirect_resolver: Arc::new(RedirectResolver::new(cache)),
|
||||||
|
@ -1296,12 +1298,10 @@ impl Documents {
|
||||||
&self.jsr_resolver
|
&self.jsr_resolver
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn refresh_jsr_resolver(
|
pub fn refresh_lockfile(&mut self, lockfile: Option<Arc<Mutex<Lockfile>>>) {
|
||||||
&mut self,
|
|
||||||
lockfile: Option<Arc<Mutex<Lockfile>>>,
|
|
||||||
) {
|
|
||||||
self.jsr_resolver =
|
self.jsr_resolver =
|
||||||
Arc::new(JsrCacheResolver::new(self.cache.clone(), lockfile));
|
Arc::new(JsrCacheResolver::new(self.cache.clone(), lockfile.clone()));
|
||||||
|
self.lockfile = lockfile;
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn update_config(
|
pub fn update_config(
|
||||||
|
@ -1339,6 +1339,7 @@ impl Documents {
|
||||||
self.cache.clone(),
|
self.cache.clone(),
|
||||||
config.tree.root_lockfile().cloned(),
|
config.tree.root_lockfile().cloned(),
|
||||||
));
|
));
|
||||||
|
self.lockfile = config.tree.root_lockfile().cloned();
|
||||||
self.redirect_resolver =
|
self.redirect_resolver =
|
||||||
Arc::new(RedirectResolver::new(self.cache.clone()));
|
Arc::new(RedirectResolver::new(self.cache.clone()));
|
||||||
let resolver = self.resolver.as_graph_resolver();
|
let resolver = self.resolver.as_graph_resolver();
|
||||||
|
@ -1494,6 +1495,19 @@ impl Documents {
|
||||||
}
|
}
|
||||||
|
|
||||||
let mut npm_reqs = doc_analyzer.npm_reqs;
|
let mut npm_reqs = doc_analyzer.npm_reqs;
|
||||||
|
|
||||||
|
// fill the reqs from the lockfile
|
||||||
|
if let Some(lockfile) = self.lockfile.as_ref() {
|
||||||
|
let lockfile = lockfile.lock();
|
||||||
|
for key in lockfile.content.packages.specifiers.keys() {
|
||||||
|
if let Some(key) = key.strip_prefix("npm:") {
|
||||||
|
if let Ok(req) = PackageReq::from_str(key) {
|
||||||
|
npm_reqs.insert(req);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
// Ensure a @types/node package exists when any module uses a node: specifier.
|
// Ensure a @types/node package exists when any module uses a node: specifier.
|
||||||
// Unlike on the command line, here we just add @types/node to the npm package
|
// Unlike on the command line, here we just add @types/node to the npm package
|
||||||
// requirements since this won't end up in the lockfile.
|
// requirements since this won't end up in the lockfile.
|
||||||
|
|
|
@ -364,7 +364,7 @@ impl LanguageServer {
|
||||||
{
|
{
|
||||||
let mut inner = self.0.write().await;
|
let mut inner = self.0.write().await;
|
||||||
let lockfile = inner.config.tree.root_lockfile().cloned();
|
let lockfile = inner.config.tree.root_lockfile().cloned();
|
||||||
inner.documents.refresh_jsr_resolver(lockfile);
|
inner.documents.refresh_lockfile(lockfile);
|
||||||
inner.refresh_npm_specifiers().await;
|
inner.refresh_npm_specifiers().await;
|
||||||
}
|
}
|
||||||
// now refresh the data in a read
|
// now refresh the data in a read
|
||||||
|
|
|
@ -310,9 +310,15 @@ async fn add_package_reqs_to_snapshot(
|
||||||
.iter()
|
.iter()
|
||||||
.all(|req| snapshot.package_reqs().contains_key(req))
|
.all(|req| snapshot.package_reqs().contains_key(req))
|
||||||
{
|
{
|
||||||
log::debug!("Snapshot already up to date. Skipping pending resolution.");
|
log::debug!(
|
||||||
|
"Snapshot already up to date. Skipping pending npm resolution."
|
||||||
|
);
|
||||||
snapshot
|
snapshot
|
||||||
} else {
|
} else {
|
||||||
|
log::debug!(
|
||||||
|
/* this string is used in tests!! */
|
||||||
|
"Running pending npm resolution."
|
||||||
|
);
|
||||||
let pending_resolver = get_npm_pending_resolver(api);
|
let pending_resolver = get_npm_pending_resolver(api);
|
||||||
let result = pending_resolver
|
let result = pending_resolver
|
||||||
.resolve_pending(snapshot, package_reqs)
|
.resolve_pending(snapshot, package_reqs)
|
||||||
|
|
|
@ -12253,3 +12253,40 @@ C.test();
|
||||||
|
|
||||||
client.shutdown();
|
client.shutdown();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn lsp_uses_lockfile_for_npm_initialization() {
|
||||||
|
let context = TestContextBuilder::for_npm().use_temp_cwd().build();
|
||||||
|
let temp_dir = context.temp_dir();
|
||||||
|
temp_dir.write("deno.json", "{}");
|
||||||
|
// use two npm packages here
|
||||||
|
temp_dir.write("main.ts", "import 'npm:@denotest/esm-basic'; import 'npm:@denotest/cjs-default-export';");
|
||||||
|
context
|
||||||
|
.new_command()
|
||||||
|
.args("run main.ts")
|
||||||
|
.run()
|
||||||
|
.skip_output_check();
|
||||||
|
// remove one of the npm packages and let the other one be found via the lockfile
|
||||||
|
temp_dir.write("main.ts", "import 'npm:@denotest/esm-basic';");
|
||||||
|
assert!(temp_dir.path().join("deno.lock").exists());
|
||||||
|
let mut client = context
|
||||||
|
.new_lsp_command()
|
||||||
|
.capture_stderr()
|
||||||
|
.log_debug()
|
||||||
|
.build();
|
||||||
|
client.initialize_default();
|
||||||
|
let mut skipping_count = 0;
|
||||||
|
client.wait_until_stderr_line(|line| {
|
||||||
|
if line.contains("Skipping pending npm resolution.") {
|
||||||
|
skipping_count += 1;
|
||||||
|
}
|
||||||
|
assert!(
|
||||||
|
!line.contains("Running pending npm resolution."),
|
||||||
|
"Line: {}",
|
||||||
|
line
|
||||||
|
);
|
||||||
|
line.contains("Server ready.")
|
||||||
|
});
|
||||||
|
assert_eq!(skipping_count, 1);
|
||||||
|
client.shutdown();
|
||||||
|
}
|
||||||
|
|
|
@ -464,6 +464,7 @@ impl InitializeParamsBuilder {
|
||||||
pub struct LspClientBuilder {
|
pub struct LspClientBuilder {
|
||||||
print_stderr: bool,
|
print_stderr: bool,
|
||||||
capture_stderr: bool,
|
capture_stderr: bool,
|
||||||
|
log_debug: bool,
|
||||||
deno_exe: PathRef,
|
deno_exe: PathRef,
|
||||||
root_dir: PathRef,
|
root_dir: PathRef,
|
||||||
use_diagnostic_sync: bool,
|
use_diagnostic_sync: bool,
|
||||||
|
@ -481,6 +482,7 @@ impl LspClientBuilder {
|
||||||
Self {
|
Self {
|
||||||
print_stderr: false,
|
print_stderr: false,
|
||||||
capture_stderr: false,
|
capture_stderr: false,
|
||||||
|
log_debug: false,
|
||||||
deno_exe: deno_exe_path(),
|
deno_exe: deno_exe_path(),
|
||||||
root_dir: deno_dir.path().clone(),
|
root_dir: deno_dir.path().clone(),
|
||||||
use_diagnostic_sync: true,
|
use_diagnostic_sync: true,
|
||||||
|
@ -507,6 +509,11 @@ impl LspClientBuilder {
|
||||||
self
|
self
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub fn log_debug(mut self) -> Self {
|
||||||
|
self.log_debug = true;
|
||||||
|
self
|
||||||
|
}
|
||||||
|
|
||||||
/// Whether to use the synchronization messages to better sync diagnostics
|
/// Whether to use the synchronization messages to better sync diagnostics
|
||||||
/// between the test client and server.
|
/// between the test client and server.
|
||||||
pub fn use_diagnostic_sync(mut self, value: bool) -> Self {
|
pub fn use_diagnostic_sync(mut self, value: bool) -> Self {
|
||||||
|
@ -537,6 +544,10 @@ impl LspClientBuilder {
|
||||||
pub fn build_result(&self) -> Result<LspClient> {
|
pub fn build_result(&self) -> Result<LspClient> {
|
||||||
let deno_dir = self.deno_dir.clone();
|
let deno_dir = self.deno_dir.clone();
|
||||||
let mut command = Command::new(&self.deno_exe);
|
let mut command = Command::new(&self.deno_exe);
|
||||||
|
let mut args = vec!["lsp".to_string()];
|
||||||
|
if self.log_debug {
|
||||||
|
args.push("--log-level=debug".to_string());
|
||||||
|
}
|
||||||
command
|
command
|
||||||
.env("DENO_DIR", deno_dir.path())
|
.env("DENO_DIR", deno_dir.path())
|
||||||
.env("NPM_CONFIG_REGISTRY", npm_registry_url())
|
.env("NPM_CONFIG_REGISTRY", npm_registry_url())
|
||||||
|
@ -547,7 +558,7 @@ impl LspClientBuilder {
|
||||||
if self.use_diagnostic_sync { "1" } else { "" },
|
if self.use_diagnostic_sync { "1" } else { "" },
|
||||||
)
|
)
|
||||||
.env("DENO_NO_UPDATE_CHECK", "1")
|
.env("DENO_NO_UPDATE_CHECK", "1")
|
||||||
.arg("lsp")
|
.args(args)
|
||||||
.stdin(Stdio::piped())
|
.stdin(Stdio::piped())
|
||||||
.stdout(Stdio::piped());
|
.stdout(Stdio::piped());
|
||||||
for (key, value) in &self.envs {
|
for (key, value) in &self.envs {
|
||||||
|
@ -651,7 +662,10 @@ impl LspClient {
|
||||||
}
|
}
|
||||||
|
|
||||||
#[track_caller]
|
#[track_caller]
|
||||||
pub fn wait_until_stderr_line(&self, condition: impl Fn(&str) -> bool) {
|
pub fn wait_until_stderr_line(
|
||||||
|
&self,
|
||||||
|
mut condition: impl FnMut(&str) -> bool,
|
||||||
|
) {
|
||||||
let timeout_time =
|
let timeout_time =
|
||||||
Instant::now().checked_add(Duration::from_secs(5)).unwrap();
|
Instant::now().checked_add(Duration::from_secs(5)).unwrap();
|
||||||
let lines_rx = self
|
let lines_rx = self
|
||||||
|
|
Loading…
Reference in a new issue