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

ci(benchmark): compare against pnpm #177

Draft
wants to merge 23 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
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
4 changes: 2 additions & 2 deletions .github/workflows/integrated-benchmark.yml
Original file line number Diff line number Diff line change
Expand Up @@ -107,14 +107,14 @@ jobs:
- name: 'Benchmark: Frozen Lockfile'
shell: bash
run: |
cargo run --bin=integrated-benchmark -- --scenario=frozen-lockfile --verdaccio HEAD main
cargo run --bin=integrated-benchmark -- --scenario=frozen-lockfile --verdaccio HEAD main --with-pnpm
cp bench-work-env/BENCHMARK_REPORT.md bench-work-env/BENCHMARK_REPORT_FROZEN_LOCKFILE.md
cp bench-work-env/BENCHMARK_REPORT.json bench-work-env/BENCHMARK_REPORT_FROZEN_LOCKFILE.json

# - name: 'Benchmark: Clean Install'
# shell: bash
# run: |
# cargo run --bin=integrated-benchmark -- --scenario=clean-install --verdaccio HEAD main
# cargo run --bin=integrated-benchmark -- --scenario=clean-install --verdaccio HEAD main --with-pnpm
# cp bench-work-env/BENCHMARK_REPORT.md bench-work-env/BENCHMARK_REPORT_CLEAN_INSTALL.md
# cp bench-work-env/BENCHMARK_REPORT.json bench-work-env/BENCHMARK_REPORT_FROZEN_LOCKFILE.json

Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

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

34 changes: 23 additions & 11 deletions crates/package-manager/src/create_virtual_dir_by_snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,29 @@
}
})?;

// 1. Install the files from `cas_paths`
let save_path =
virtual_node_modules_dir.join(dependency_path.package_specifier.name.to_string());
create_cas_files(import_method, &save_path, cas_paths)
.map_err(CreateVirtualDirError::CreateCasFiles)?;
let mut result1: Result<(), CreateVirtualDirError> = Ok(());
let mut result2: Result<(), CreateVirtualDirError> = Ok(());
rayon::scope(|scope| {
// 1. Install the files from `cas_paths`
scope.spawn(|_| {
let save_path = virtual_node_modules_dir
.join(dependency_path.package_specifier.name.to_string());
result1 = create_cas_files(import_method, &save_path, cas_paths)
.map_err(CreateVirtualDirError::CreateCasFiles);
});

Check warning on line 69 in crates/package-manager/src/create_virtual_dir_by_snapshot.rs

View check run for this annotation

Codecov / codecov/patch

crates/package-manager/src/create_virtual_dir_by_snapshot.rs#L60-L69

Added lines #L60 - L69 were not covered by tests

// 2. Create the symlink layout
if let Some(dependencies) = &package_snapshot.dependencies {
create_symlink_layout(dependencies, virtual_store_dir, &virtual_node_modules_dir)
}

Ok(())
// 2. Create the symlink layout
scope.spawn(|_| {
if let Some(dependencies) = &package_snapshot.dependencies {
create_symlink_layout(
dependencies,
virtual_store_dir,
&virtual_node_modules_dir,
);
result2 = Ok(());
}
})
});
result1.and(result2)

Check warning on line 83 in crates/package-manager/src/create_virtual_dir_by_snapshot.rs

View check run for this annotation

Codecov / codecov/patch

crates/package-manager/src/create_virtual_dir_by_snapshot.rs#L71-L83

Added lines #L71 - L83 were not covered by tests
}
}
19 changes: 5 additions & 14 deletions crates/package-manager/src/create_virtual_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,13 @@
use futures_util::future;
use pacquet_lockfile::{DependencyPath, PackageSnapshot, RootProjectSnapshot};
use pacquet_npmrc::Npmrc;
use pacquet_tarball::Cache;
use pipe_trait::Pipe;
use reqwest::Client;
use std::collections::HashMap;

/// This subroutine generates filesystem layout for the virtual store at `node_modules/.pacquet`.
#[must_use]
pub struct CreateVirtualStore<'a> {
pub tarball_cache: &'a Cache,
pub http_client: &'a Client,
pub config: &'static Npmrc,
pub packages: Option<&'a HashMap<DependencyPath, PackageSnapshot>>,
Expand All @@ -20,8 +18,7 @@
impl<'a> CreateVirtualStore<'a> {
/// Execute the subroutine.
pub async fn run(self) {
let CreateVirtualStore { tarball_cache, http_client, config, packages, project_snapshot } =
self;
let CreateVirtualStore { http_client, config, packages, project_snapshot } = self;

Check warning on line 21 in crates/package-manager/src/create_virtual_store.rs

View check run for this annotation

Codecov / codecov/patch

crates/package-manager/src/create_virtual_store.rs#L21

Added line #L21 was not covered by tests

let packages = packages.unwrap_or_else(|| {
dbg!(project_snapshot);
Expand All @@ -31,16 +28,10 @@
packages
.iter()
.map(|(dependency_path, package_snapshot)| async move {
InstallPackageBySnapshot {
tarball_cache,
http_client,
config,
dependency_path,
package_snapshot,
}
.run()
.await
.unwrap(); // TODO: properly propagate this error
InstallPackageBySnapshot { http_client, config, dependency_path, package_snapshot }
.run()
.await
.unwrap(); // TODO: properly propagate this error

Check warning on line 34 in crates/package-manager/src/create_virtual_store.rs

View check run for this annotation

Codecov / codecov/patch

crates/package-manager/src/create_virtual_store.rs#L31-L34

Added lines #L31 - L34 were not covered by tests
})
.pipe(future::join_all)
.await;
Expand Down
1 change: 0 additions & 1 deletion crates/package-manager/src/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ where
assert_eq!(lockfile_version.major, 6); // compatibility check already happens at serde, but this still helps preventing programmer mistakes.

InstallFrozenLockfile {
tarball_cache,
http_client,
config,
project_snapshot,
Expand Down
7 changes: 1 addition & 6 deletions crates/package-manager/src/install_frozen_lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
use pacquet_lockfile::{DependencyPath, PackageSnapshot, RootProjectSnapshot};
use pacquet_npmrc::Npmrc;
use pacquet_package_manifest::DependencyGroup;
use pacquet_tarball::Cache;
use reqwest::Client;
use std::collections::HashMap;

Expand All @@ -20,7 +19,6 @@
where
DependencyGroupList: IntoIterator<Item = DependencyGroup>,
{
pub tarball_cache: &'a Cache,
pub http_client: &'a Client,
pub config: &'static Npmrc,
pub project_snapshot: &'a RootProjectSnapshot,
Expand All @@ -35,7 +33,6 @@
/// Execute the subroutine.
pub async fn run(self) {
let InstallFrozenLockfile {
tarball_cache,
http_client,
config,
project_snapshot,
Expand All @@ -47,9 +44,7 @@

assert!(config.prefer_frozen_lockfile, "Non frozen lockfile is not yet supported");

CreateVirtualStore { tarball_cache, http_client, config, packages, project_snapshot }
.run()
.await;
CreateVirtualStore { http_client, config, packages, project_snapshot }.run().await;

Check warning on line 47 in crates/package-manager/src/install_frozen_lockfile.rs

View check run for this annotation

Codecov / codecov/patch

crates/package-manager/src/install_frozen_lockfile.rs#L47

Added line #L47 was not covered by tests

SymlinkDirectDependencies { config, project_snapshot, dependency_groups }.run();
}
Expand Down
15 changes: 4 additions & 11 deletions crates/package-manager/src/install_package_by_snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use miette::Diagnostic;
use pacquet_lockfile::{DependencyPath, LockfileResolution, PackageSnapshot, PkgNameVerPeer};
use pacquet_npmrc::Npmrc;
use pacquet_tarball::{Cache, DownloadTarballToStore, TarballError};
use pacquet_tarball::{DownloadTarballToStore, TarballError};
use pipe_trait::Pipe;
use reqwest::Client;
use std::borrow::Cow;
Expand All @@ -12,7 +12,6 @@
/// then creates the symlink layout for the package.
#[must_use]
pub struct InstallPackageBySnapshot<'a> {
pub tarball_cache: &'a Cache,
pub http_client: &'a Client,
pub config: &'static Npmrc,
pub dependency_path: &'a DependencyPath,
Expand All @@ -29,13 +28,8 @@
impl<'a> InstallPackageBySnapshot<'a> {
/// Execute the subroutine.
pub async fn run(self) -> Result<(), InstallPackageBySnapshotError> {
let InstallPackageBySnapshot {
tarball_cache,
http_client,
config,
dependency_path,
package_snapshot,
} = self;
let InstallPackageBySnapshot { http_client, config, dependency_path, package_snapshot } =
self;

Check warning on line 32 in crates/package-manager/src/install_package_by_snapshot.rs

View check run for this annotation

Codecov / codecov/patch

crates/package-manager/src/install_package_by_snapshot.rs#L31-L32

Added lines #L31 - L32 were not covered by tests
let PackageSnapshot { resolution, .. } = package_snapshot;
let DependencyPath { custom_registry, package_specifier } = dependency_path;

Expand Down Expand Up @@ -64,14 +58,13 @@

// TODO: skip when already exists in store?
let cas_paths = DownloadTarballToStore {
tarball_cache,
http_client,
store_dir: &config.store_dir,
package_integrity: integrity,
package_unpacked_size: None,
package_url: &tarball_url,
}
.run()
.without_cache()

Check warning on line 67 in crates/package-manager/src/install_package_by_snapshot.rs

View check run for this annotation

Codecov / codecov/patch

crates/package-manager/src/install_package_by_snapshot.rs#L67

Added line #L67 was not covered by tests
.await
.map_err(InstallPackageBySnapshotError::DownloadTarball)?;

Expand Down
3 changes: 1 addition & 2 deletions crates/package-manager/src/install_package_from_registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,6 @@ impl<'a> InstallPackageFromRegistry<'a> {

// TODO: skip when it already exists in store?
let cas_paths = DownloadTarballToStore {
tarball_cache,
http_client,
store_dir: &config.store_dir,
package_integrity: package_version
Expand All @@ -86,7 +85,7 @@ impl<'a> InstallPackageFromRegistry<'a> {
package_unpacked_size: package_version.dist.unpacked_size,
package_url: package_version.as_tarball_url(),
}
.run()
.with_cache(tarball_cache)
.await
.map_err(InstallPackageFromRegistryError::DownloadTarballToStore)?;

Expand Down
1 change: 1 addition & 0 deletions crates/tarball/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ dashmap = { workspace = true }
derive_more = { workspace = true }
miette = { workspace = true }
pipe-trait = { workspace = true }
rayon = { workspace = true }
reqwest = { workspace = true }
serde = { workspace = true }
serde_json = { workspace = true }
Expand Down
Loading