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

Exhibit A: perf with rayon #186

Draft
wants to merge 25 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
e73d651
ci(benchmark): compare against pnpm
KSXGitHub Nov 3, 2023
efdc69d
refactor: parse integrity eagerly
KSXGitHub Nov 3, 2023
3dfd353
docs: clarify
KSXGitHub Nov 3, 2023
16e89c8
docs: remove the 3rd option
KSXGitHub Nov 3, 2023
8289e36
chore(benchmark): add even more packages
KSXGitHub Nov 3, 2023
25cf3e6
fix: autoInstallPeers
KSXGitHub Nov 3, 2023
6ea62e8
perf: parallelize 2 steps
KSXGitHub Nov 3, 2023
8f12c87
perf: ignore cache for frozen-lockfile
KSXGitHub Nov 3, 2023
816e4bc
docs: add a todo
KSXGitHub Nov 3, 2023
5b203a7
refactor: remove unnecessary `pipe_ref`
KSXGitHub Nov 4, 2023
9037d99
chore(git): merge
KSXGitHub Nov 4, 2023
4fa537f
feat(tarball): only use cache when required
KSXGitHub Nov 4, 2023
36e278e
docs: remove outdated todo
KSXGitHub Nov 4, 2023
585a6a6
chore(git): merge from main
KSXGitHub Nov 4, 2023
d35637f
perf(tarball): separate extract from checksum
KSXGitHub Nov 4, 2023
219e019
perf(tarball): wrap integrity in an Arc
KSXGitHub Nov 4, 2023
fbcca1f
feat(tarball): log after extraction
KSXGitHub Nov 4, 2023
fc7d2b1
refactor: remove unnecessary Arc
KSXGitHub Nov 4, 2023
ccd661e
refactor: only use cache when required
KSXGitHub Nov 4, 2023
17eccda
refactor: remove unnecessary Arc
KSXGitHub Nov 4, 2023
690d5e1
chore(git): merge
KSXGitHub Nov 4, 2023
9c38751
perf(tarball): write_cas_file in parallel
KSXGitHub Nov 4, 2023
cf7bb55
perf(tarball): use read_to_end
KSXGitHub Nov 4, 2023
1371383
chore(git): revert unrelated commit
KSXGitHub Nov 10, 2023
1f4b10f
chore(git): revert unrelated commit
KSXGitHub Nov 10, 2023
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
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 @@ impl<'a> CreateVirtualDirBySnapshot<'a> {
}
})?;

// 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);
});

// 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)
}
}
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 crate::InstallPackageBySnapshot;
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 @@ pub struct CreateVirtualStore<'a> {
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;

let packages = packages.unwrap_or_else(|| {
dbg!(project_snapshot);
Expand All @@ -31,16 +28,10 @@ impl<'a> CreateVirtualStore<'a> {
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
})
.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 crate::{CreateVirtualStore, SymlinkDirectDependencies};
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 @@ pub struct InstallFrozenLockfile<'a, DependencyGroupList>
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 @@ where
/// Execute the subroutine.
pub async fn run(self) {
let InstallFrozenLockfile {
tarball_cache,
http_client,
config,
project_snapshot,
Expand All @@ -47,9 +44,7 @@ where

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;

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 derive_more::{Display, Error};
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 @@ use std::borrow::Cow;
/// 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 @@ pub enum InstallPackageBySnapshotError {
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;
let PackageSnapshot { resolution, .. } = package_snapshot;
let DependencyPath { custom_registry, package_specifier } = dependency_path;

Expand Down Expand Up @@ -64,14 +58,13 @@ impl<'a> InstallPackageBySnapshot<'a> {

// 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()
.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