Skip to content

Commit

Permalink
buck2: create_unhashed_outputs: do cheap checks before expensive ones
Browse files Browse the repository at this point in the history
Summary:
Its not very clear to me why we need this, but it looks like checking the count
of outputs on an artifact can take a while in aggregate. I guess this makes
sense since we can have large artifact groups as other-outputs?

Anyway, it's cheaper to check whether the artifact is indeed a default output
(which we care about) before attempting to iterate than it is to do the
reverse, so we might as well.

Reviewed By: nshipilov

Differential Revision: D39425072

fbshipit-source-id: 1897f16d60ea80583a227617d423823d0fe8af1d
  • Loading branch information
krallin authored and facebook-github-bot committed Sep 12, 2022
1 parent 6b53bc2 commit d677e41
Showing 1 changed file with 15 additions and 15 deletions.
30 changes: 15 additions & 15 deletions buck2_server_commands/src/commands/build/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -287,25 +287,25 @@ fn create_unhashed_outputs(
// The following IndexMap will contain a key of the unhashed/symlink path and values of all the hashed locations that map to the unhashed location.
let mut unhashed_to_hashed: IndexMap<AbsPathBuf, HashSet<AbsPathBuf>> = IndexMap::new();
for provider_artifact in provider_artifacts {
if !matches!(provider_artifact.provider_type, BuildProviderType::Default) {
continue;
}

match provider_artifact.values.iter().exactly_one() {
Ok((artifact, _)) => match provider_artifact.provider_type {
BuildProviderType::Default => match artifact.as_parts().0 {
BaseArtifactKind::Build(build) => {
let unhashed_path =
artifact_fs.retrieve_unhashed_location(build.get_path());
let path = artifact_fs.resolve(artifact.get_path())?;
let abs_unhashed_path = fs.resolve(&unhashed_path);
let entry = unhashed_to_hashed
.entry(abs_unhashed_path)
.or_insert_with(HashSet::new);
entry.insert(fs.resolve(&path));
}
_ => {}
},
Ok((artifact, _)) => match artifact.as_parts().0 {
BaseArtifactKind::Build(build) => {
let unhashed_path = artifact_fs.retrieve_unhashed_location(build.get_path());
let path = artifact_fs.resolve(artifact.get_path())?;
let abs_unhashed_path = fs.resolve(&unhashed_path);
let entry = unhashed_to_hashed
.entry(abs_unhashed_path)
.or_insert_with(HashSet::new);
entry.insert(fs.resolve(&path));
}
_ => {}
},
Err(_) => {}
}
};
}
// The IndexMap is used now to determine if and what conflicts exist where multiple hashed artifact locations
// all want a symlink to the same unhashed artifact location and deal with them accordingly.
Expand Down

0 comments on commit d677e41

Please sign in to comment.