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

perf: skip npm install if graph has no new packages #24017

Merged
merged 24 commits into from
May 28, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Update deno_lockfile
  • Loading branch information
dsherret committed May 27, 2024
commit 82f03c5eaaeb991992efbf3634b9c1f800d8c099
8 changes: 5 additions & 3 deletions Cargo.lock

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

3 changes: 0 additions & 3 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -369,6 +369,3 @@ opt-level = 3
opt-level = 3
[profile.release.package.base64-simd]
opt-level = 3

[patch.crates-io]
eszip = { path = "../eszip" }
2 changes: 1 addition & 1 deletion cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ deno_runtime = { workspace = true, features = ["include_js_files_for_snapshottin
deno_semver = "=0.5.4"
deno_task_shell = "=0.16.1"
deno_terminal.workspace = true
eszip = "=0.70.0"
eszip = "=0.70.1"
napi_sym.workspace = true

async-trait.workspace = true
Expand Down
22 changes: 21 additions & 1 deletion cli/args/lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@

use std::path::PathBuf;

use deno_core::anyhow::Context;
use deno_core::error::AnyError;
use deno_runtime::deno_node::PackageJson;

use crate::args::ConfigFile;
use crate::cache;
use crate::util::fs::atomic_write_file;
use crate::Flags;

use super::DenoSubcommand;
Expand Down Expand Up @@ -53,6 +56,23 @@ pub fn discover(
},
};

let lockfile = Lockfile::new(filename, flags.lock_write)?;
let lockfile = if flags.lock_write {
Lockfile::new_empty(filename, true)
} else {
let file_text = std::fs::read_to_string(&filename).with_context(|| {
format!("Failed reading lockfile '{}'", filename.display())
})?;
Lockfile::with_lockfile_content(filename, &file_text, false)?
};
Ok(Some(lockfile))
}

pub fn write_lockfile_if_has_changes(
lockfile: &Lockfile,
) -> Result<(), AnyError> {
let Some(bytes) = lockfile.resolve_write_bytes() else {
return Ok(()); // nothing to do
};
atomic_write_file(&lockfile.filename, bytes, cache::CACHE_PERM)
.context("Failed writing lockfile.")
}
1 change: 1 addition & 0 deletions cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ pub use deno_config::TsConfigType;
pub use deno_config::TsTypeLib;
pub use deno_config::WorkspaceConfig;
pub use flags::*;
pub use lockfile::write_lockfile_if_has_changes;
pub use lockfile::Lockfile;
pub use package_json::PackageJsonDepsProvider;

Expand Down
9 changes: 8 additions & 1 deletion cli/lsp/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use deno_ast::MediaType;
use deno_config::FmtOptionsConfig;
use deno_config::TsConfig;
use deno_core::anyhow::anyhow;
use deno_core::error::AnyError;
use deno_core::parking_lot::Mutex;
use deno_core::serde::de::DeserializeOwned;
use deno_core::serde::Deserialize;
Expand Down Expand Up @@ -1685,7 +1686,13 @@ fn resolve_node_modules_dir(
}

fn resolve_lockfile_from_path(lockfile_path: PathBuf) -> Option<Lockfile> {
match Lockfile::new(lockfile_path, false) {
let result = std::fs::read_to_string(&lockfile_path)
.map_err(AnyError::from)
.and_then(|text| {
Lockfile::with_lockfile_content(lockfile_path, &text, false)
.map_err(AnyError::from)
});
match result {
Ok(value) => {
if let Ok(specifier) = ModuleSpecifier::from_file_path(&value.filename) {
lsp_log!(" Resolved lockfile: \"{}\"", specifier);
Expand Down
5 changes: 3 additions & 2 deletions cli/lsp/language_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ use super::tsc::TsServer;
use super::urls;
use crate::args::create_default_npmrc;
use crate::args::get_root_cert_store;
use crate::args::write_lockfile_if_has_changes;
use crate::args::CaData;
use crate::args::CacheSetting;
use crate::args::CliOptions;
Expand Down Expand Up @@ -260,8 +261,8 @@ impl LanguageServer {
// found after caching
if let Some(lockfile) = cli_options.maybe_lockfile() {
let lockfile = lockfile.lock();
if let Err(err) = lockfile.write() {
lsp_warn!("Error writing lockfile: {:#}", err);
if let Err(err) = write_lockfile_if_has_changes(&lockfile) {
lsp_warn!("{:#}", err);
}
}

Expand Down
6 changes: 3 additions & 3 deletions cli/module_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use std::str;
use std::sync::Arc;

use crate::args::jsr_url;
use crate::args::write_lockfile_if_has_changes;
use crate::args::CliOptions;
use crate::args::DenoSubcommand;
use crate::args::TsTypeLib;
Expand Down Expand Up @@ -172,11 +173,10 @@ impl ModuleLoadPreparer {

self.module_graph_builder.graph_roots_valid(graph, roots)?;

// If there is a lockfile...
// write the lockfile if there is one
if let Some(lockfile) = &self.lockfile {
let lockfile = lockfile.lock();
// update it with anything new
lockfile.write().context("Failed writing lockfile.")?;
write_lockfile_if_has_changes(&lockfile)?;
}

drop(_pb_clear_guard);
Expand Down
1 change: 0 additions & 1 deletion cli/npm/managed/resolution.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,6 @@ fn npm_package_to_lockfile_info(
.collect();

NpmPackageLockfileInfo {
display_id: pkg.id.nv.to_string(),
serialized_id: pkg.id.as_serialized(),
integrity: pkg.dist.integrity().for_lockfile(),
dependencies,
Expand Down
5 changes: 2 additions & 3 deletions cli/tools/info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use std::fmt::Write;

use deno_ast::ModuleSpecifier;
use deno_core::anyhow::bail;
use deno_core::anyhow::Context;
use deno_core::error::AnyError;
use deno_core::resolve_url_or_path;
use deno_core::serde_json;
Expand All @@ -26,6 +25,7 @@ use deno_semver::npm::NpmPackageReqReference;
use deno_semver::package::PackageNv;
use deno_terminal::colors;

use crate::args::write_lockfile_if_has_changes;
use crate::args::Flags;
use crate::args::InfoFlags;
use crate::display;
Expand Down Expand Up @@ -70,8 +70,7 @@ pub async fn info(flags: Flags, info_flags: InfoFlags) -> Result<(), AnyError> {
// If there is a lockfile...
if let Some(lockfile) = &maybe_lockfile {
let lockfile = lockfile.lock();
// update it with anything new
lockfile.write().context("Failed writing lockfile.")?;
write_lockfile_if_has_changes(&lockfile)?;
}

if info_flags.json {
Expand Down
3 changes: 2 additions & 1 deletion cli/tools/installer.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright 2018-2024 the Deno authors. All rights reserved. MIT license.

use crate::args::resolve_no_prompt;
use crate::args::write_lockfile_if_has_changes;
use crate::args::AddFlags;
use crate::args::CaData;
use crate::args::Flags;
Expand Down Expand Up @@ -266,7 +267,7 @@ async fn install_local(
crate::module_loader::load_top_level_deps(&factory).await?;

if let Some(lockfile) = factory.cli_options().maybe_lockfile() {
lockfile.lock().write()?;
write_lockfile_if_has_changes(&lockfile.lock())?;
}

Ok(())
Expand Down
7 changes: 2 additions & 5 deletions cli/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use std::sync::Arc;

use deno_ast::ModuleSpecifier;
use deno_core::anyhow::bail;
use deno_core::anyhow::Context;
use deno_core::error::AnyError;
use deno_core::futures::FutureExt;
use deno_core::parking_lot::Mutex;
Expand Down Expand Up @@ -48,6 +47,7 @@ use deno_terminal::colors;
use tokio::select;

use crate::args::package_json::PackageJsonDeps;
use crate::args::write_lockfile_if_has_changes;
use crate::args::DenoSubcommand;
use crate::args::StorageKeyResolver;
use crate::errors;
Expand Down Expand Up @@ -533,10 +533,7 @@ impl CliMainWorkerFactory {
// For npm binary commands, ensure that the lockfile gets updated
// so that we can re-use the npm resolution the next time it runs
// for better performance
lockfile
.lock()
.write()
.context("Failed writing lockfile.")?;
write_lockfile_if_has_changes(&lockfile.lock())?;
}

(node_resolution.into_url(), is_main_cjs)
Expand Down