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

nrwl · Jul 28, 2023 · 6d341e0 · 6d341e0
1 parent 4c2f98e
commit 6d341e0
Show file tree

Hide file tree

Showing 3 changed files with 37 additions and 26 deletions.
diff --git a/packages/nx/src/hasher/__snapshots__/task-hasher.spec.ts.snap b/packages/nx/src/hasher/__snapshots__/task-hasher.spec.ts.snap
@@ -63,7 +63,7 @@ exports[`TaskHasher hashTarget should hash entire subtree of dependencies 1`] =
     },
     "runtime": {},
   },
-  "value": "17607022607820563118",
+  "value": "14419327228911184578",
 }
 `;
 
@@ -83,7 +83,7 @@ exports[`TaskHasher hashTarget should hash executor dependencies of @nx packages
     },
     "runtime": {},
   },
-  "value": "15096054768893599383",
+  "value": "379625642227035180",
 }
 `;
 
@@ -105,7 +105,7 @@ exports[`TaskHasher hashTarget should use externalDependencies to override nx:ru
     },
     "runtime": {},
   },
-  "value": "18142315317355318287",
+  "value": "14779270419297346086",
 }
 `;
 
@@ -239,7 +239,7 @@ exports[`TaskHasher should hash missing dependent npm project versions 1`] = `
     },
     "runtime": {},
   },
-  "value": "3668827038634092448",
+  "value": "13210933885500739919",
 }
 `;
 
@@ -321,7 +321,7 @@ exports[`TaskHasher should hash npm project versions 1`] = `
     },
     "runtime": {},
   },
-  "value": "3668827038634092448",
+  "value": "13210933885500739919",
 }
 `;
 

diff --git a/packages/nx/src/hasher/task-hasher.spec.ts b/packages/nx/src/hasher/task-hasher.spec.ts
@@ -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: {},

diff --git a/packages/nx/src/hasher/task-hasher.ts b/packages/nx/src/hasher/task-hasher.ts
@@ -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(
@@ -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 };
   }
@@ -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,
@@ -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(