mirror of
https://github.com/abhigyanpatwari/GitNexus.git
synced 2026-08-28 05:25:25 +00:00
* perf(import-target): index the workspace once per run for go/csharp/dart/ruby Four import-target resolvers answered their lookups with a full `allFilePaths` scan per import, making resolution O(imports x files): - go (#2877): `findRootPackageFiles` / `findAllFilesInPkgDir`, the latter once per path segment on the GOPATH fallback. Most Go imports are external, so the whole cascade ran to completion before returning null. - csharp (#2878): the no-csproj leg took the raw Set past the memoized index the csproj leg was already using - up to eight passes for a four-segment `using`. - dart (#2879): one scan per candidate path, and for an external package both candidates miss, so both always ran to completion. - ruby (#2880): a complete `buildSuffixIndex` rebuilt and discarded per `require` - every require paid to index every file in the repo. Each now reads an index memoized on the `allFilePaths` Set identity, the shape `getPythonFileIndex` (#1918) and csharp's own `getWorkspaceFileIndex` (#1881) already used. Two shared modules back them: - `workspace-file-index.ts`: normalized list + `SuffixIndex` + a normalized->raw map, for csharp and ruby. - `package-dir-index.ts`: "which files live directly inside a directory ending with <path>", for go and csharp. Candidates are bucketed by the directory's last segment rather than by indexing every directory suffix, which would cost O(files x depth) entries at kernel scale (#2649). Behaviour is unchanged, including the tie-breaks that are expressed only through Set-iteration order and `indexOf` positions: the go root leg stays sorted and its package leg stays unsorted, the first-occurrence rule that excludes a directory nested inside a same-named directory is preserved, csharp's whole-path match still beats an earlier suffix match, and dart still tries `lib/<rel>` fully before bare `<rel>` and matches raw paths. Verified two ways. `import-target-index-parity.test.ts` keeps verbatim copies of the pre-change implementations and diffs against them over a deterministic corpus plus hand-built layouts for each tie-break; six mutations of the new code were confirmed to fail it. Separately, the bench corpus produces byte-identical fingerprints against the pre-change resolvers at both 400 and 1600 files. `bench/import-target/measure.mjs` gates both arms in CI: per-language output fingerprints, a scaling budget (measured 0.98-1.12 here, 3.32-4.10 against the pre-change scans), and the corpus shape, so the corpus cannot be shrunk below the size the scaling arm needs and still print PASS. Closes #2877 Closes #2878 Closes #2879 Closes #2880 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BCptYZWRgnnJ821rzebQyj * perf(import-target): cover kotlin, and add depth + absolute-cost arms #2872 landed the same index hoist for Kotlin while this branch was open. Fold it into the shared measures so all five resolvers are gated on one corpus, and adopt the two arms that PR's review proved a scaling ratio alone cannot carry. - `bench/import-target/measure.mjs` gains a kotlin arm: a Gradle-shaped corpus with per-module source roots over one package namespace, `.kt` and `.kts` stems, a nested same-name package directory, and a share of wildcard `.*` imports so the package fan-out tier — the only tier whose output is order-bearing — is inside the fingerprint. - `depth_ratio`: deep arm at a FIXED file count with ~6x the path components. `scaling_ratio` divides the file count out, so it is scale-invariant and structurally cannot see a cost that grows with path depth instead, and `buildSuffixIndex` (C#, Ruby) and Kotlin's `suffixByStem` each emit one entry per component. Measured: go 0.98, dart 0.88 (depth-free indexes), ruby 1.48, kotlin 2.20, csharp 3.45 — which is why the budget is per language. One global budget would have to sit at 5.0 and would let Dart go 0.88 -> 4.9 unnoticed. - `small_ms_ceiling`: an absolute bound at 4x the measured arm, because a constant-factor regression that grows both scale arms equally passes every ratio. - The deep arm must resolve exactly what the small arm resolves. Padding was supposed to change depth and nothing else; a deep arm that stopped resolving would be timing the null path. The five fingerprints are unchanged by this commit - verified against the previous baseline before rewriting it, so adding the kotlin arm and the deep scale did not perturb the four languages' output. Kotlin joins the Set-iteration counter in `import-target-index-parity.test.ts` too. Its own guard (`kotlin-import-index-reuse.test.ts`) counts index BUILDS, which a scan added beside a reused index does not move. That counter is also the only DETERMINISTIC guard against a reintroduced scan, and this commit documents why rather than pretending otherwise: a full workspace scan on 1-in-32 imports was measured to pass every timing arm here (dart, 1.458 scaling against a 1.8 budget, 1.736 ms against a 4 ms ceiling) while the counter reads 14 instead of 1. Tightening the ceilings toward the noise floor to chase that case would only buy flaky CI. `bench/kotlin-import-target/` stays: it fingerprints both file-set iteration orders and probes the four-tier cascade shape by shape, neither of which this corpus does. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BCptYZWRgnnJ821rzebQyj * perf(import-target): merge matching package dirs in one pass `filesDirectlyInPkgDir` re-spread its accumulator once per matching directory, costing O(files x dirs^2) copies per import. On a Go monorepo where many services carry the same package directory (`svcN/internal/pkg`, which Go's GOPATH cascade queries by two-segment tail) that made the index SLOWER than the scan it replaced: 13.4x at 1600 matching directories. Append into one array and sort once. Measured against a verbatim copy of the pre-change scan, output byte-identical at every k: k=200 1400 files old 0.126 ms was 0.169 ms now 0.042 ms k=800 5600 files old 0.457 ms was 3.002 ms now 0.185 ms k=1600 11200 files old 0.960 ms was 12.890 ms now 0.232 ms The index now beats the scan by 2.5-4.1x on this shape instead of losing to it by up to 13x. Also drop the min-`ord` comparison in `firstFileDirectlyInPkgDir`: the build loop appends a directory to its last-segment bucket the moment it accepts that directory's first file, so bucket order already IS ascending first-file-`ord` order and the first hit is the minimum. Differentially verified at 0 divergences. The invariant, and the build-loop edits that would silently break it, are now recorded at the early return. Type the index containers as deeply readonly so Go's deliberate `[...rootFiles].sort()` copy is compile-enforced rather than comment-enforced, and correct the header's claim that a polyglot repo "never pays" -- only the stored index is per-language. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rLy781K8E3rRHFGEe1khJ * test(import-target): guard index reuse at the adapter boundary `workspace-file-index.ts` documented the hazard as "a defensive `new Set(allFilePaths)` in an ADAPTER" -- the bug #1918 shipped -- and named the unit parity test as the guard. It is not: that test imports the resolvers directly, while production reaches them through `<lang>ScopeResolver.resolveImportTarget`. Inserting the copy at `go/scope-resolver.ts:31`, `csharp:35`, `dart:189` and `ruby:268` left the parity test 28/28 green and `measure.mjs --check` PASS in all four cases. Kotlin and Python already had adapter-level guards; go/csharp/dart/ruby had none. Add `test/integration/<lang>-import-index-reuse.test.ts` for the four, mirroring the Kotlin/Python precedent: resolve through the scope resolver, assert the file set is traversed once (twice for C#, which builds two indexes), and pair every count with a result assertion so a count of 1 cannot be the count of an adapter that resolves nothing. Each was proven to fail under the copy it exists to catch: go expected 600 to be 1 dart expected 600 to be 1 ruby expected 400 to be 1 csharp expected 600 to be 2 `CountingSet` moves to `test/helpers/counting-file-set.ts` and now counts `forEach`, `values`, `keys` and `entries` as well as `[Symbol.iterator]`. It missed a rescan spelled `allFilePaths.forEach(...)` entirely; with the overrides that mutation reads 14 instead of 1. Four fixtures that pinned the guard next door, each now shown to kill its mutation: - the Dart "matched RAW" case used a forward-slash target, so the basename bucket missed before the raw comparison was reached and it asserted `null === null`. A positive twin carrying the backslash in the TARGET catches both half-mutations. - no C# or Ruby target addressed the corpus's `win\dir\thing` file, so deleting the backslash normalization in `workspace-file-index.ts` passed both gates. Now 4 failures. - `normToRaw`'s first-wins rule had no normalization twin in any corpus. - the Go nested-package fixture was decided by the `endsWith` half and never reached the first-occurrence branch its title names; addressing the directory as a single segment makes it reach it. The parity test's own docstring no longer claims the scan count is a complete census -- it names the three materialized arrays it cannot see. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rLy781K8E3rRHFGEe1khJ * test(import-target): assert every scale, add collide and retained-heap arms Three holes in the gate this PR ships as its own proof. 1. `--check` computed three fingerprints per language, stored all three, and compared one. `DEEP_PAD = 16 -> 0` deleted the entire depth arm and still printed PASS, because depth padding is count-neutral by design so no asserted number moved. Assert `fingerprint` per scale, and assert `deep.fingerprint !== small.fingerprint` so the padding's EFFECT is pinned, not just its output. 2. The corpus minted per-index directory names (`src/pkg${d}`, `src/Ns${d}`, `lib/feature${d}`), so max last-segment bucket and max matching dirs were both 1 -- and bucket cardinality is the only non-constant term the index has. The `dirCount > 1` merge branch had never executed in any arm. Add a `collide` arm on shared-leaf layouts with identical files/imports/resolved counts; it reaches 9,269 multi-directory merges per run, up to 34 directories at once. Go and C#/Dart legitimately score above the linear budget there and get their own; Ruby and Kotlin stay at 1.8 because their keyed maps are collision-immune and that immunity is the assertion. 3. No arm measured memory, while the C# no-csproj leg newly retains an O(files x depth) suffix index. Add a retained-heap arm on the `bench/cfg` pattern, including its loud failure when `--expose-gc` is missing rather than a silent skip. Measured at 32k files: csharp 73.62 MiB, ruby 55.26 MiB. Ceiling is 1.5x, NOT the 4x the timing arms use -- the measurement is byte-stable to 0.00085% across processes, so 4x would be throwing away the gate. `_arms_note` records why, so nobody harmonises it back. `depth_ratio`, added by this PR, flaked ~1-in-20: go peaked at 1.748 and dart at 2.043 against a 1.6 budget, both ratios of two sub-3 ms minima. Fixed at the estimator, not the threshold -- REPS 5 -> 15, matching `bench/cfg`, `schema-pairs` and `callable-value-flow` (5 was the lowest in the repo; the sibling `kotlin-import-target` uses 7, which was not enough here). 22/22 PASS, every arm now at 70-78% of its budget with a <=1.26x swing. No budget was widened; the distributions are recorded in `_arms_note` so the headroom is visibly earned. Three copies of the same overclaim corrected: the parity test NARROWS the 1-in-32 blind spot, it does not close it -- it watches the Set while the resolvers hold materialized arrays. `_floor` no longer claims its ratios "match" the issues' (different corpora, both quadratic). The step moves to the END of the benchmarks job and runs with `--expose-gc`. A failing step aborts every step after it (#2895), so the newest, least-proven gate must not sit ahead of eight established ones. All five output fingerprints are byte-identical to before this session -- the proof that every change here was behaviour-preserving. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rLy781K8E3rRHFGEe1khJ * docs(import-target): point the reuse contract at the guard that guards it `workspace-file-index.ts` told callers the unit parity test guards the adapter-copy hazard. It does not -- it never crosses the adapter. Name both layers and say which catches what: the per-language `test/integration/<lang>-import-index-reuse.test.ts` files at the adapter boundary, the parity test for a rescan reintroduced inside a resolver. The C# namespace-dir index comment named `findDirectChild`, which this PR deleted; it feeds `firstFileDirectlyInPkgDir` now. Drop `GoResolveContext`, dead since the legacy call-resolution DAG was removed in #942 -- zero importers, and `gitnexus`'s package.json declares no `main`, `exports` or `types`, so it is not a published surface. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013rLy781K8E3rRHFGEe1khJ * refactor(import-target): quality pass over the review-round changes Four cleanup lanes (reuse / simplification / efficiency / altitude) over the previous four commits. No behaviour change anywhere: all 25 bench cells (5 languages x 5 arms) are byte-identical on files, imports, resolved, distinct_outcomes and fingerprint, re-verified after each individual edit. **Restores a fast path the last commit lost.** Fixing the O(k^2) accumulator made the SINGLE-directory case — the overwhelmingly common one — copy the bucket where the original aliased it: measured 1.11x slower at 4 files/dir rising to 1.72x at 128. Holding the first bucket by reference and promoting to an accumulator only when a second directory appears is 0.65-0.97x of the previous code at dirCount=1 and parity at dirCount=64. 176-case differential, 0 divergences. **`sortedRootFiles` accessor.** `rootFiles` was the only index container read directly from outside the module. `readonly` is erased at runtime and `Array.isArray` widens it back, so the copy rule now lives with the code that owns the invariant instead of at the call site. No `Object.freeze`: V8's PACKED_FROZEN_ELEMENTS read cost lands on the hot `matchingDirs` path. **One shared arm for the four reuse guards.** The distinct-file-set test was copy-pasted four ways, 33-38 identical lines each, and this repo's own helpers (`mini-repo.ts`, `scope-model.ts`) document extracting at the SECOND verbatim consumer. `expectDistinctFileSetsGetOwnIndex` takes what actually varies; its `expected` type excludes `null` so the pairing rule cannot be reinstated as a hole. The per-language first and third arms stay duplicated on purpose — corpora and payload shapes genuinely differ. Re-proven: all four still fail under an adapter-inserted `new Set(allFilePaths)`. **Bench.** `dirsFor` shared by the two functions that must agree on directory fan-out (they mint and address the same files). `SCALES` derived from the arm table, so a future arm cannot be measured, printed and silently never asserted. Five timing checks with one shape collapsed to a table — the trailing sentence had already drifted into four wordings. `uniqueTarget`/`collideTarget` as flat functions, mirroring the `uniqueDir`/`collideDir` split rather than nesting a second axis four ternaries deep. One `identityPass` replaces two untimed full resolution passes per cell: -371 ms median. **CI step moved back where it belongs.** It was parked last "until #2895 lands", but that reasoning was backwards twice over: the flake that motivated it was fixed at the estimator in the previous commit, and #2895's own audit measured the last slot as executing zero times in 13 runs. It sits with the other resolver-index guards; #2899 carries the `if: !cancelled()` that fixes step masking for every step at once. Filed rather than fixed here: #2908 (java and cobol still scan the workspace per import, same shape as #2877-#2880, neither memoized), #2909 (make index reuse a contract test over SCOPE_RESOLVERS on one instrument). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Co58j4au9JLf8dmJpwdF9B --------- Co-authored-by: Gergo Magyar <gergomagyar0@gmail.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
161 lines
6.8 KiB
TypeScript
161 lines
6.8 KiB
TypeScript
/**
|
|
* A `Set<string>` that counts how many times it is TRAVERSED in full — the
|
|
* measuring instrument behind the import-target index-reuse guards
|
|
* (`test/unit/scope-resolution/import-target-index-parity.test.ts` and the
|
|
* per-language `test/integration/<lang>-import-index-reuse.test.ts` files).
|
|
*
|
|
* ## Why a counting Set rather than a production build counter
|
|
*
|
|
* Kotlin and Python count index BUILDS from production (`languages/<lang>/
|
|
* index-stats.ts`). That catches the per-import rebuild, but it is blind to a
|
|
* scan added BESIDE a reused index: the cache still hits, the build count still
|
|
* reads 1. Counting traversals of the file set instead needs no production
|
|
* surface at all and catches both failures with one number:
|
|
*
|
|
* - an adapter that copies the set (`new Set(allFilePaths)`) hands a fresh
|
|
* `WeakMap` key per import, so the count rises to the import count;
|
|
* - a scan reintroduced next to the index raises the count by one per scan.
|
|
*
|
|
* The second is the case the benchmark provably cannot see: a full workspace
|
|
* scan on 1-in-32 imports passes every timing arm in `bench/import-target/`
|
|
* (measured, see `baselines.json` `_blind_spot`) while this counter reads 14
|
|
* instead of 1.
|
|
*
|
|
* ## Why every traversal entry point is overridden
|
|
*
|
|
* `for…of`, spread and `new Set(x)` go through `[Symbol.iterator]`, but
|
|
* `forEach`, `values`, `keys` and `entries` walk the same elements without
|
|
* touching it. Overriding only `[Symbol.iterator]` would let a reintroduced
|
|
* scan spelled `allFilePaths.forEach(…)` or `[...allFilePaths.values()]` sit
|
|
* under the guard uncounted. `Set.prototype[Symbol.iterator]` and
|
|
* `Set.prototype.values` are the same function object in the spec, but the
|
|
* `super.*` lookups below resolve on `Set.prototype`, not on this subclass, so
|
|
* a single traversal is still counted exactly once.
|
|
*
|
|
* ## What it does NOT see
|
|
*
|
|
* Only traversals of the SET. Once an index has materialized the file list into
|
|
* an array (`WorkspaceFileIndex.normalized` / `.all`, Dart's basename buckets,
|
|
* `PackageDirIndex.filesByDir`), a scan over that array is invisible here.
|
|
* Guarding that would mean either instrumenting production or proxying an index
|
|
* internal; see the header of the parity test for why neither is in place.
|
|
*
|
|
* `instanceof Set` still holds, which matters: C#'s `narrowContext` rejects a
|
|
* workspace context whose `allFilePaths` is not a `Set`, so a plain object with
|
|
* a counter would silently resolve nothing and every assertion would pass on
|
|
* `null === null`.
|
|
*
|
|
* `expectDistinctFileSetsGetOwnIndex` below is the one arm of those guards that
|
|
* is identical in every language once the four values that differ are named, so
|
|
* it lives here beside the instrument it reads rather than in each guard.
|
|
*/
|
|
import { expect } from 'vitest';
|
|
import type { ScopeResolver } from '../../src/core/ingestion/scope-resolution/contract/scope-resolver.js';
|
|
|
|
export class CountingSet extends Set<string> {
|
|
/** Full traversals of this set, by any entry point. */
|
|
scans = 0;
|
|
|
|
override [Symbol.iterator](): SetIterator<string> {
|
|
this.scans++;
|
|
return super[Symbol.iterator]();
|
|
}
|
|
|
|
override values(): SetIterator<string> {
|
|
this.scans++;
|
|
return super.values();
|
|
}
|
|
|
|
override keys(): SetIterator<string> {
|
|
this.scans++;
|
|
return super.keys();
|
|
}
|
|
|
|
override entries(): SetIterator<[string, string]> {
|
|
this.scans++;
|
|
return super.entries();
|
|
}
|
|
|
|
override forEach(
|
|
callbackfn: (value: string, value2: string, set: Set<string>) => void,
|
|
thisArg?: unknown,
|
|
): void {
|
|
this.scans++;
|
|
super.forEach(callbackfn, thisArg);
|
|
}
|
|
}
|
|
|
|
/**
|
|
* Everything that differs between the per-language spellings of the
|
|
* distinct-file-set arm. Nothing else about that arm varies, which is why it is
|
|
* a parameter list rather than four copies.
|
|
*/
|
|
export interface DistinctFileSetArm {
|
|
/**
|
|
* The orchestrator adapter under test — `<lang>ScopeResolver
|
|
* .resolveImportTarget`, NOT the language's `resolve<Lang>ImportTarget`. The
|
|
* adapter is the surface the unit parity test cannot reach, and the surface a
|
|
* defensive `new Set(allFilePaths)` copy would break.
|
|
*/
|
|
readonly resolveImportTarget: ScopeResolver['resolveImportTarget'];
|
|
/**
|
|
* Builds one workspace. Called twice, and must return a FRESH `CountingSet`
|
|
* each time: the two sets being distinct objects is the whole subject of the
|
|
* arm, since the indexes are memoized on Set identity via a `WeakMap`.
|
|
*/
|
|
readonly buildWorkspace: () => CountingSet;
|
|
/** The import spelling to resolve, as it would appear in source. */
|
|
readonly targetRaw: string;
|
|
/** The importing file the target is resolved against. */
|
|
readonly fromFile: string;
|
|
/**
|
|
* The resolver's `resolutionConfig` argument (Go's `{ modulePath }`). Not
|
|
* optional: the languages that take none pass `undefined` in the open, so a
|
|
* call site never hides which adapters read this channel behind an omission.
|
|
*/
|
|
readonly resolutionConfig: unknown;
|
|
/**
|
|
* What `targetRaw` must resolve to — a path for the string-returning
|
|
* resolvers, a path list for Go. Never `null`: the pairing rule below exists
|
|
* precisely because an adapter that has stopped resolving anything returns
|
|
* `null` and still posts a perfect scan count, so a `null` expectation would
|
|
* reinstate the hole it closes.
|
|
*/
|
|
readonly expected: string | readonly string[];
|
|
/** Traversals of ONE file set that full reuse permits: one per index built. */
|
|
readonly expectedScans: number;
|
|
}
|
|
|
|
/** Resolutions driven against each file set before the counts are read. */
|
|
const DISTINCT_FILE_SET_REPEATS = 20;
|
|
|
|
/**
|
|
* Assert that two independently built file sets each get their own index, built
|
|
* once — no stale reuse of one set's index for the other, and no rebuild per
|
|
* import within either.
|
|
*
|
|
* The repeats are driven bare, following the equivalent arm in
|
|
* `test/unit/scope-resolution/import-target-index-parity.test.ts`: asserting
|
|
* inside the loop restates one bit of information forty times. The two asserted
|
|
* resolutions afterwards are the pairing rule the guards' headers state — a
|
|
* scan count must never be the count of an adapter that resolves nothing.
|
|
*/
|
|
export function expectDistinctFileSetsGetOwnIndex(arm: DistinctFileSetArm): void {
|
|
const a = arm.buildWorkspace();
|
|
const b = arm.buildWorkspace();
|
|
|
|
for (let i = 0; i < DISTINCT_FILE_SET_REPEATS; i++) {
|
|
arm.resolveImportTarget(arm.targetRaw, arm.fromFile, a, arm.resolutionConfig);
|
|
arm.resolveImportTarget(arm.targetRaw, arm.fromFile, b, arm.resolutionConfig);
|
|
}
|
|
|
|
expect(arm.resolveImportTarget(arm.targetRaw, arm.fromFile, a, arm.resolutionConfig)).toEqual(
|
|
arm.expected,
|
|
);
|
|
expect(arm.resolveImportTarget(arm.targetRaw, arm.fromFile, b, arm.resolutionConfig)).toEqual(
|
|
arm.expected,
|
|
);
|
|
|
|
expect(a.scans).toBe(arm.expectedScans);
|
|
expect(b.scans).toBe(arm.expectedScans);
|
|
}
|