perf(javascript): defer ambiguity expansion in version comparisons (#2962)
perf(javascript): avoid quadratic temporary matching state Repeated source groups can allocate candidates that later identity matches discard. Retain the original ambiguous groups until all tiers finish, expand only unmatched survivors, and append to locally owned key buckets in place. Constraint: Preserve matching priority, full ambiguity, and public output. Directive: Do not re-match residual groups that become unique after later tiers. Tested: 59 relevant tests, npm run check, and test discovery on current main. Tested: CLI benchmarks and full-suite failure comparison on the recorded base. Not-tested: Full suite on the updated main; native Linux and Windows workflows.
This commit is contained in:
Shenrui Ma
GitHub
parent
0e6eca4085
commit
388b907c92
|
|
@ -77,15 +77,23 @@ export const matchJavaScriptApplicationVersions = (
|
|||
): ApplicationVersionMatchingProjection => {
|
||||
const left = new Map(leftNodes.map((node) => [node.node_id, node]));
|
||||
const right = new Map(rightNodes.map((node) => [node.node_id, node]));
|
||||
const leftCandidates = new Map<string, Set<string>>();
|
||||
const rightCandidates = new Map<string, Set<string>>();
|
||||
const pairs: ApplicationVersionNodePair[] = [];
|
||||
const ambiguousGroups: AmbiguousGroup[] = [];
|
||||
for (const tier of matchTiers())
|
||||
applyTier({ left, right, pairs, leftCandidates, rightCandidates, tier });
|
||||
applyTier({ left, right, pairs, ambiguousGroups, tier });
|
||||
const remainingLeft = sortedNodes(left.values());
|
||||
const remainingRight = sortedNodes(right.values());
|
||||
const leftIds = new Set(remainingLeft.map(({ node_id: id }) => id));
|
||||
const rightIds = new Set(remainingRight.map(({ node_id: id }) => id));
|
||||
const leftCandidates = new Map<string, Set<string>>();
|
||||
const rightCandidates = new Map<string, Set<string>>();
|
||||
// Later tiers can pair members of an ambiguous group. Expand only survivors,
|
||||
// without pairing groups that became unique after those later matches.
|
||||
for (const group of ambiguousGroups) {
|
||||
const leftGroup = group.left.filter((node) => left.has(node.node_id));
|
||||
if (leftGroup.length === 0) continue;
|
||||
const rightGroup = group.right.filter((node) => right.has(node.node_id));
|
||||
if (rightGroup.length === 0) continue;
|
||||
recordCandidates(leftGroup, rightGroup, leftCandidates, rightCandidates);
|
||||
}
|
||||
return {
|
||||
pairs: pairs.sort((a, b) =>
|
||||
compareUnicodeCodePoints(
|
||||
|
|
@ -95,17 +103,21 @@ export const matchJavaScriptApplicationVersions = (
|
|||
),
|
||||
unmatchedLeft: remainingLeft,
|
||||
unmatchedRight: remainingRight,
|
||||
candidatesForLeft: filteredCandidates(leftCandidates, rightIds),
|
||||
candidatesForRight: filteredCandidates(rightCandidates, leftIds),
|
||||
candidatesForLeft: sortedCandidates(leftCandidates),
|
||||
candidatesForRight: sortedCandidates(rightCandidates),
|
||||
};
|
||||
};
|
||||
|
||||
interface AmbiguousGroup {
|
||||
readonly left: readonly ApplicationNode[];
|
||||
readonly right: readonly ApplicationNode[];
|
||||
}
|
||||
|
||||
interface TierContext {
|
||||
readonly left: Map<string, ApplicationNode>;
|
||||
readonly right: Map<string, ApplicationNode>;
|
||||
readonly pairs: ApplicationVersionNodePair[];
|
||||
readonly leftCandidates: Map<string, Set<string>>;
|
||||
readonly rightCandidates: Map<string, Set<string>>;
|
||||
readonly ambiguousGroups: AmbiguousGroup[];
|
||||
readonly tier: MatchTier;
|
||||
}
|
||||
|
||||
|
|
@ -114,15 +126,12 @@ const applyTier = (context: TierContext): void => {
|
|||
const rightByKey = nodesByKey(context.right.values(), context.tier.key);
|
||||
for (const key of [...leftByKey.keys()].sort(compareUnicodeCodePoints)) {
|
||||
const leftGroup = leftByKey.get(key) ?? [];
|
||||
const rightGroup = rightByKey.get(key) ?? [];
|
||||
if (rightGroup.length === 0) continue;
|
||||
recordCandidates(
|
||||
leftGroup,
|
||||
rightGroup,
|
||||
context.leftCandidates,
|
||||
context.rightCandidates,
|
||||
);
|
||||
if (leftGroup.length !== 1 || rightGroup.length !== 1) continue;
|
||||
const rightGroup = rightByKey.get(key);
|
||||
if (rightGroup === undefined) continue;
|
||||
if (leftGroup.length !== 1 || rightGroup.length !== 1) {
|
||||
context.ambiguousGroups.push({ left: leftGroup, right: rightGroup });
|
||||
continue;
|
||||
}
|
||||
const leftNode = leftGroup[0];
|
||||
const rightNode = rightGroup[0];
|
||||
if (leftNode === undefined || rightNode === undefined) continue;
|
||||
|
|
@ -144,7 +153,9 @@ const nodesByKey = (
|
|||
for (const node of nodes) {
|
||||
const key = keyFor(node);
|
||||
if (key === null) continue;
|
||||
output.set(key, [...(output.get(key) ?? []), node]);
|
||||
const group = output.get(key);
|
||||
if (group === undefined) output.set(key, [node]);
|
||||
else group.push(node);
|
||||
}
|
||||
return output;
|
||||
};
|
||||
|
|
@ -169,17 +180,14 @@ const recordCandidates = (
|
|||
}
|
||||
};
|
||||
|
||||
const filteredCandidates = (
|
||||
const sortedCandidates = (
|
||||
candidates: ReadonlyMap<string, ReadonlySet<string>>,
|
||||
retained: ReadonlySet<string>,
|
||||
): ReadonlyMap<string, readonly string[]> =>
|
||||
new Map(
|
||||
[...candidates.entries()].flatMap(([nodeId, values]) => {
|
||||
const filtered = [...values]
|
||||
.filter((value) => retained.has(value))
|
||||
.sort(compareUnicodeCodePoints);
|
||||
return filtered.length === 0 ? [] : [[nodeId, filtered] as const];
|
||||
}),
|
||||
[...candidates.entries()].map(([nodeId, values]) => [
|
||||
nodeId,
|
||||
[...values].sort(compareUnicodeCodePoints),
|
||||
]),
|
||||
);
|
||||
|
||||
const nodeIdentityKey = (node: ApplicationNode): string =>
|
||||
|
|
|
|||
|
|
@ -0,0 +1,181 @@
|
|||
import { describe, expect, it } from "vitest";
|
||||
|
||||
import {
|
||||
artifactEvidence,
|
||||
completeCoverage,
|
||||
fixtureNode,
|
||||
} from "./javascriptApplicationGraph.fixture.js";
|
||||
import {
|
||||
createJavaScriptApplicationGraph,
|
||||
type JavaScriptApplicationGraph,
|
||||
} from "./javascriptApplicationGraph.js";
|
||||
import type { ApplicationNode } from "./javascriptApplicationGraphSchemas.js";
|
||||
import { compareJavaScriptApplicationVersions } from "./javascriptApplicationVersionComparison.js";
|
||||
|
||||
const ARTIFACT_SHA = "a".repeat(64);
|
||||
|
||||
const moduleNode = (name: string, source: string, structure?: string) =>
|
||||
fixtureNode(
|
||||
"javascript-module",
|
||||
{
|
||||
strategy: "canonical-path",
|
||||
stability: "artifact-version",
|
||||
artifact_sha256: ARTIFACT_SHA,
|
||||
path: `${name}.js`,
|
||||
},
|
||||
name,
|
||||
{
|
||||
source_sha256: source.repeat(64),
|
||||
...(structure === undefined
|
||||
? {}
|
||||
: {
|
||||
structural_fingerprint_sha256: structure.repeat(64),
|
||||
structural_fingerprint_algorithm: "fixture",
|
||||
}),
|
||||
},
|
||||
artifactEvidence(ARTIFACT_SHA, `${name}.js`),
|
||||
);
|
||||
|
||||
const graph = (nodes: ApplicationNode[]): JavaScriptApplicationGraph =>
|
||||
createJavaScriptApplicationGraph({
|
||||
schema: "JavaScriptApplicationGraph",
|
||||
root_node_ids: nodes.map(({ node_id }) => node_id),
|
||||
nodes,
|
||||
edges: [],
|
||||
coverage: completeCoverage,
|
||||
limitations: [],
|
||||
});
|
||||
|
||||
const compare = (
|
||||
left: JavaScriptApplicationGraph,
|
||||
right: JavaScriptApplicationGraph,
|
||||
) =>
|
||||
compareJavaScriptApplicationVersions({
|
||||
left: {
|
||||
evidenceId: `ev_${"1".repeat(64)}`,
|
||||
rootArtifactSha256: ARTIFACT_SHA,
|
||||
graph: left,
|
||||
},
|
||||
right: {
|
||||
evidenceId: `ev_${"2".repeat(64)}`,
|
||||
rootArtifactSha256: ARTIFACT_SHA,
|
||||
graph: right,
|
||||
},
|
||||
leftNativeEvidence: [],
|
||||
rightNativeEvidence: [],
|
||||
});
|
||||
|
||||
describe("application version matching", () => {
|
||||
it("does not revisit a source group that becomes unique after identity matching", () => {
|
||||
const shared = moduleNode("shared", "1");
|
||||
const leftOnly = moduleNode("left", "1");
|
||||
const rightOnly = moduleNode("right", "1");
|
||||
const result = compare(
|
||||
graph([shared, leftOnly]),
|
||||
graph([shared, rightOnly]),
|
||||
);
|
||||
|
||||
expect(result.matching).toMatchObject({
|
||||
exact_node_identity: 1,
|
||||
exact_module_source_digest: 0,
|
||||
ambiguous: 2,
|
||||
unmatched: 0,
|
||||
});
|
||||
expect(
|
||||
result.items.find(({ left_node_id }) => left_node_id === shared.node_id),
|
||||
).toMatchObject({
|
||||
right_node_id: shared.node_id,
|
||||
match: { status: "matched", basis: "exact-node-identity" },
|
||||
});
|
||||
expect(
|
||||
result.items.find(
|
||||
({ left_node_id }) => left_node_id === leftOnly.node_id,
|
||||
),
|
||||
).toMatchObject({
|
||||
status: "unknown",
|
||||
right_node_id: null,
|
||||
match: {
|
||||
status: "ambiguous",
|
||||
candidate_right_node_ids: [rightOnly.node_id],
|
||||
},
|
||||
});
|
||||
expect(
|
||||
result.items.find(
|
||||
({ right_node_id }) => right_node_id === rightOnly.node_id,
|
||||
),
|
||||
).toMatchObject({
|
||||
status: "unknown",
|
||||
left_node_id: null,
|
||||
match: {
|
||||
status: "ambiguous",
|
||||
candidate_left_node_ids: [leftOnly.node_id],
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it("unions all unresolved source and structural candidates without forcing pairs", () => {
|
||||
const a = moduleNode("a", "1", "a");
|
||||
const b = moduleNode("b", "1", "a");
|
||||
const c = moduleNode("c", "2", "a");
|
||||
const x = moduleNode("x", "1", "a");
|
||||
const y = moduleNode("y", "1", "b");
|
||||
const z = moduleNode("z", "3", "a");
|
||||
const left = graph([a, b, c]);
|
||||
const right = graph([x, y, z]);
|
||||
const result = compare(left, right);
|
||||
|
||||
expect(result.summary).toEqual({
|
||||
unchanged: 0,
|
||||
added: 0,
|
||||
removed: 0,
|
||||
changed: 0,
|
||||
unknown: 6,
|
||||
});
|
||||
expect(result.matching).toMatchObject({ ambiguous: 6, unmatched: 0 });
|
||||
for (const [node, candidates] of [
|
||||
[a, [x, y, z]],
|
||||
[b, [x, y, z]],
|
||||
[c, [x, z]],
|
||||
] as const) {
|
||||
expect(
|
||||
result.items.find(({ left_node_id }) => left_node_id === node.node_id)
|
||||
?.match,
|
||||
).toEqual({
|
||||
status: "ambiguous",
|
||||
basis: "none",
|
||||
confidence: "unknown",
|
||||
candidate_left_node_ids: [],
|
||||
candidate_right_node_ids: candidates
|
||||
.map(({ node_id }) => node_id)
|
||||
.sort(),
|
||||
});
|
||||
}
|
||||
for (const [node, candidates] of [
|
||||
[x, [a, b, c]],
|
||||
[y, [a, b]],
|
||||
[z, [a, b, c]],
|
||||
] as const) {
|
||||
expect(
|
||||
result.items.find(({ right_node_id }) => right_node_id === node.node_id)
|
||||
?.match,
|
||||
).toEqual({
|
||||
status: "ambiguous",
|
||||
basis: "none",
|
||||
confidence: "unknown",
|
||||
candidate_left_node_ids: candidates
|
||||
.map(({ node_id }) => node_id)
|
||||
.sort(),
|
||||
candidate_right_node_ids: [],
|
||||
});
|
||||
}
|
||||
|
||||
// Permute the already validated graph so matching sees the changed order.
|
||||
// Compare the full result, including item, comparison and change-graph IDs.
|
||||
expect(
|
||||
compare(
|
||||
{ ...left, nodes: [...left.nodes].reverse() },
|
||||
{ ...right, nodes: [...right.nodes].reverse() },
|
||||
),
|
||||
).toEqual(result);
|
||||
});
|
||||
});
|
||||
|
|
@ -0,0 +1,98 @@
|
|||
import { mkdir, writeFile } from "node:fs/promises";
|
||||
import { join } from "node:path";
|
||||
|
||||
import { expect } from "vitest";
|
||||
|
||||
import { parseEvidence } from "../../../src/domain/evidence.js";
|
||||
import { javascriptApplicationAnalysisResultSchema } from "../../../src/domain/javascript/javascriptApplicationAnalysis.js";
|
||||
import { applicationVersionComparisonResultSchema } from "../../../src/domain/javascript/javascriptApplicationVersionComparisonSchemas.js";
|
||||
import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
|
||||
import { cliTest } from "../../support/cli/cliFixture.js";
|
||||
|
||||
cliTest(
|
||||
"pairs repeated factory bodies by identity when application versions retain a shared bundle",
|
||||
async ({ cli }) => {
|
||||
const root = await createTestTempDirectory("rea-version-matching-");
|
||||
const factoryCount = 32;
|
||||
const factories = Array.from(
|
||||
{ length: factoryCount },
|
||||
(_, index) => `${index}: function() {}`,
|
||||
).join(",");
|
||||
const bundle = `globalThis.webpackChunkApp.push([[1], {${factories}}]);`;
|
||||
const versions = await Promise.all(
|
||||
[1, 2].map(async (version) => {
|
||||
const application = join(root, String(version));
|
||||
await mkdir(application);
|
||||
await writeFile(
|
||||
join(application, "package.json"),
|
||||
JSON.stringify({ name: "shared-bundle", version: `${version}.0.0` }),
|
||||
);
|
||||
await writeFile(join(application, "chunk.js"), bundle);
|
||||
const response = await cli.run({
|
||||
arguments: ["analyze-javascript-application", application, "--json"],
|
||||
environment: { REA_LOG_LEVEL: "silent" },
|
||||
});
|
||||
expect(response.exitCode, response.stderr).toBe(0);
|
||||
return parseEvidence(response.json);
|
||||
}),
|
||||
);
|
||||
const [left, right] = versions;
|
||||
if (left === undefined || right === undefined)
|
||||
throw new Error("Missing application version Evidence");
|
||||
const analyses = versions.map(({ normalized_result }) =>
|
||||
javascriptApplicationAnalysisResultSchema.parse(normalized_result),
|
||||
);
|
||||
const factoryNodes = analyses.map(({ graph }) => {
|
||||
expect(graph.coverage.status).toBe("complete");
|
||||
const nodes = graph.nodes.filter(
|
||||
({ kind, observations }) =>
|
||||
kind === "javascript-module" &&
|
||||
observations.some(
|
||||
({ properties }) => properties.module_key !== undefined,
|
||||
),
|
||||
);
|
||||
expect(nodes).toHaveLength(factoryCount);
|
||||
const sourceDigests = nodes.flatMap(({ observations }) =>
|
||||
observations.map(({ properties }) => properties.source_sha256),
|
||||
);
|
||||
expect(sourceDigests.every((digest) => typeof digest === "string")).toBe(
|
||||
true,
|
||||
);
|
||||
expect(new Set(sourceDigests).size).toBe(1);
|
||||
return nodes;
|
||||
});
|
||||
const inputPath = join(root, "comparison.json");
|
||||
await writeFile(inputPath, JSON.stringify({ left, right }));
|
||||
const response = await cli.run({
|
||||
arguments: ["compare-application-versions", inputPath, "--json"],
|
||||
environment: { REA_LOG_LEVEL: "silent" },
|
||||
});
|
||||
expect(response.exitCode, response.stderr).toBe(0);
|
||||
const result = applicationVersionComparisonResultSchema.parse(
|
||||
parseEvidence(response.json).normalized_result,
|
||||
);
|
||||
|
||||
expect(result.left.root_artifact_sha256).not.toBe(
|
||||
result.right.root_artifact_sha256,
|
||||
);
|
||||
expect(result.matching.ambiguous).toBe(0);
|
||||
expect(result.coverage.status).toBe("complete-within-inputs");
|
||||
expect(factoryNodes[0]?.map(({ node_id }) => node_id)).toEqual(
|
||||
factoryNodes[1]?.map(({ node_id }) => node_id),
|
||||
);
|
||||
for (const node of factoryNodes[0] ?? []) {
|
||||
expect(
|
||||
result.items.find(({ left_node_id }) => left_node_id === node.node_id),
|
||||
).toMatchObject({
|
||||
status: "unchanged",
|
||||
right_node_id: node.node_id,
|
||||
match: {
|
||||
status: "matched",
|
||||
basis: "exact-node-identity",
|
||||
candidate_left_node_ids: [],
|
||||
candidate_right_node_ids: [],
|
||||
},
|
||||
});
|
||||
}
|
||||
},
|
||||
);
|
||||
Loading…
Reference in New Issue