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

fix(core): fix performance of hashing external dependencies up front #18359

Merged
merged 1 commit into from
Jul 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
10 changes: 5 additions & 5 deletions packages/nx/src/hasher/__snapshots__/task-hasher.spec.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ exports[`TaskHasher hashTarget should hash entire subtree of dependencies 1`] =
},
"runtime": {},
},
"value": "17607022607820563118",
"value": "14419327228911184578",
}
`;

Expand All @@ -83,7 +83,7 @@ exports[`TaskHasher hashTarget should hash executor dependencies of @nx packages
},
"runtime": {},
},
"value": "15096054768893599383",
"value": "379625642227035180",
}
`;

Expand All @@ -105,7 +105,7 @@ exports[`TaskHasher hashTarget should use externalDependencies to override nx:ru
},
"runtime": {},
},
"value": "18142315317355318287",
"value": "14779270419297346086",
}
`;

Expand Down Expand Up @@ -239,7 +239,7 @@ exports[`TaskHasher should hash missing dependent npm project versions 1`] = `
},
"runtime": {},
},
"value": "3668827038634092448",
"value": "13210933885500739919",
}
`;

Expand Down Expand Up @@ -321,7 +321,7 @@ exports[`TaskHasher should hash npm project versions 1`] = `
},
"runtime": {},
},
"value": "3668827038634092448",
"value": "13210933885500739919",
}
`;

Expand Down
8 changes: 4 additions & 4 deletions packages/nx/src/hasher/task-hasher.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -976,23 +976,23 @@ describe('TaskHasher', () => {
const hasher1 = createHasher();
const hasher2 = createHasher();

const hashA1 = hasher1.hashTask({
const hashA1 = await hasher1.hashTask({
id: 'a-build',
target: { project: 'a', target: 'build' },
overrides: {},
});
const hashB1 = hasher1.hashTask({
const hashB1 = await hasher1.hashTask({
id: 'b-build',
target: { project: 'b', target: 'build' },
overrides: {},
});

const hashB2 = hasher2.hashTask({
const hashB2 = await hasher2.hashTask({
id: 'b-build',
target: { project: 'b', target: 'build' },
overrides: {},
});
const hashA2 = hasher2.hashTask({
const hashA2 = await hasher2.hashTask({
id: 'a-build',
target: { project: 'a', target: 'build' },
overrides: {},
Expand Down
45 changes: 28 additions & 17 deletions packages/nx/src/hasher/task-hasher.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,9 @@ class TaskHasherImpl {
private runtimeHashes: {
[runtime: string]: Promise<PartialHash>;
} = {};
private externalDependencyHashes: Map<string, PartialHash> = new Map<
private externalDependencyHashes: Map<string, PartialHash[]> = new Map<
string,
PartialHash
PartialHash[]
>();
private allExternalDependenciesHash: PartialHash;
private projectRootMappings = createProjectRootMappings(
Expand Down Expand Up @@ -300,11 +300,16 @@ class TaskHasherImpl {
}

private combinePartialHashes(partialHashes: PartialHash[]): PartialHash {
let details = {};
if (partialHashes.length === 1) {
return partialHashes[0];
}
const details = {};
const hashValues: string[] = [];
for (const partial of partialHashes) {
details = { ...details, ...partial.details };
hashValues.push(partial.value);
Object.assign(details, partial.details);
}
const value = hashArray(partialHashes.map(({ value }) => value));
const value = hashArray(hashValues);

return { value, details };
}
Expand Down Expand Up @@ -416,33 +421,37 @@ class TaskHasherImpl {
}

private getExternalDependencyHash(externalNodeName: string) {
return this.externalDependencyHashes.get(externalNodeName);
const combinedHash = this.combinePartialHashes(
this.externalDependencyHashes.get(externalNodeName)
);
// Set the combined hash into the hashes so it's not recalculated next time
this.externalDependencyHashes.set(externalNodeName, [combinedHash]);
return combinedHash;
}

private hashExternalDependency(
externalNodeName: string,
visited: Set<string>
): PartialHash {
): PartialHash[] {
// try to retrieve the hash from cache
if (this.externalDependencyHashes.has(externalNodeName)) {
return this.externalDependencyHashes.get(externalNodeName);
}
visited.add(externalNodeName);
const node = this.projectGraph.externalNodes[externalNodeName];
let partialHash: PartialHash;
const partialHashes: Set<PartialHash> = new Set<PartialHash>();
if (node) {
const partialHashes: PartialHash[] = [];
if (node.data.hash) {
// we already know the hash of this dependency
partialHashes.push({
partialHashes.add({
value: node.data.hash,
details: {
[externalNodeName]: node.data.hash,
},
});
} else {
// we take version as a hash
partialHashes.push({
partialHashes.add({
value: node.data.version,
details: {
[externalNodeName]: node.data.version,
Expand All @@ -453,25 +462,27 @@ class TaskHasherImpl {
if (this.projectGraph.dependencies[externalNodeName]) {
this.projectGraph.dependencies[externalNodeName].forEach((d) => {
if (!visited.has(d.target)) {
partialHashes.push(this.hashExternalDependency(d.target, visited));
for (const hash of this.hashExternalDependency(d.target, visited)) {
partialHashes.add(hash);
}
}
});
}
partialHash = this.combinePartialHashes(partialHashes);
} else {
// unknown dependency
// this may occur if dependency is not an npm package
// but rather symlinked in node_modules or it's pointing to a remote git repo
// in this case we have no information about the versioning of the given package
partialHash = {
partialHashes.add({
value: `__${externalNodeName}__`,
details: {
[externalNodeName]: `__${externalNodeName}__`,
},
};
});
}
this.externalDependencyHashes.set(externalNodeName, partialHash);
return partialHash;
const partialHashArray = Array.from(partialHashes);
this.externalDependencyHashes.set(externalNodeName, partialHashArray);
return partialHashArray;
}

private hashTarget(
Expand Down