Skip to content

Commit

Permalink
perf(yarn3): eagerly construct map from workspace paths to locators (#…
Browse files Browse the repository at this point in the history
…6797)

### Description

When adding support for Yarn3's resolutions, we added a lookup to find
the workspace locator so we could check if there were any overrides.

The way this was implemented was lazily looking through the locators for
one which had the range of the form `workspace:path/to/workspace`. This
was a linear search that got performed for each workspace in the
repository e.g. O(n^2).

This PR changes to eagerly build up this map to provide a quick lookup.
This happens during our initial iteration of the packages in the
lockfile. We keep the old inefficient behavior as fallback just in case
it covered some cases that the new eager approach misses i.e. a
workspace with a locator of the form
`my-package@workspace:something^now/comes/the/path` (I do not think
locators like this are produced by Yarn, but this can give confidence
that we aren't breaking users with this change)

### Testing Instructions

👀 

Traces from calculating all transitive closures in a repo with 1000
workspaces.
Main

[slow.trace.zip](https://github.com/vercel/turbo/files/13679445/slow.trace.zip)
With PR

[fast.trace.zip](https://github.com/vercel/turbo/files/13679447/fast.trace.zip)



Closes TURBO-1936
  • Loading branch information
chris-olszewski authored Dec 15, 2023
1 parent 02692b0 commit 126518a
Showing 1 changed file with 24 additions and 6 deletions.
30 changes: 24 additions & 6 deletions crates/turborepo-lockfiles/src/berry/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,8 @@ pub struct BerryLockfile {
extensions: HashSet<Descriptor<'static>>,
// Package overrides
overrides: Map<Resolution, String>,
// Map from workspace paths to package locators
workspace_path_to_locator: HashMap<String, Locator<'static>>,
}

// This is the direct representation of the lockfile as it appears on disk.
Expand Down Expand Up @@ -113,6 +115,7 @@ impl BerryLockfile {
let mut locator_package = Map::new();
let mut descriptor_locator = Map::new();
let mut resolver = DescriptorResolver::default();
let mut workspace_path_to_locator = HashMap::new();
for (key, package) in &lockfile.packages {
let locator = Locator::try_from(package.resolution.as_str())?;

Expand All @@ -125,6 +128,10 @@ impl BerryLockfile {

locator_package.insert(locator.as_owned(), package.clone());

if let Some(path) = locator.reference.strip_prefix("workspace:") {
workspace_path_to_locator.insert(path.to_string(), locator.as_owned());
}

for descriptor in Descriptor::from_lockfile_key(key) {
let descriptor = descriptor?;
if let Some(other) = resolver.insert(&descriptor) {
Expand All @@ -150,6 +157,7 @@ impl BerryLockfile {
patches,
overrides,
extensions: Default::default(),
workspace_path_to_locator,
};

this.populate_extensions()?;
Expand Down Expand Up @@ -340,6 +348,7 @@ impl BerryLockfile {
resolver: self.resolver.clone(),
extensions: self.extensions.clone(),
overrides: self.overrides.clone(),
workspace_path_to_locator: self.workspace_path_to_locator.clone(),
})
}

Expand Down Expand Up @@ -369,6 +378,19 @@ impl BerryLockfile {
// TODO Could we dedupe and wrap in Rc?
Ok(dependency.into_owned())
}

fn locator_for_workspace_path(&self, workspace_path: &str) -> Option<&Locator> {
self.workspace_path_to_locator
.get(workspace_path)
.or_else(|| {
// This is an inefficient fallback we use in case our old logic was catching
// edge cases that the eager approach misses.
self.locator_package.keys().find(|locator| {
locator.reference.starts_with("workspace:")
&& locator.reference.ends_with(workspace_path)
})
})
}
}

impl Lockfile for BerryLockfile {
Expand All @@ -383,12 +405,7 @@ impl Lockfile for BerryLockfile {
// In practice, this is extremely silly since changing the version of
// the dependency in the workspace's package.json does the same thing.
let workspace_locator = self
.locator_package
.keys()
.find(|locator| {
locator.reference.starts_with("workspace:")
&& locator.reference.ends_with(workspace_path)
})
.locator_for_workspace_path(workspace_path)
.ok_or_else(|| crate::Error::MissingWorkspace(workspace_path.to_string()))?;

let dependency = self
Expand Down Expand Up @@ -537,6 +554,7 @@ impl Lockfile for BerryLockfile {
resolver: self.resolver.clone(),
extensions: self.extensions.clone(),
overrides: self.overrides.clone(),
workspace_path_to_locator: self.workspace_path_to_locator.clone(),
}))
}

Expand Down

0 comments on commit 126518a

Please sign in to comment.