Skip to content

Commit b460153

Browse files
test(lint): index the access-matrix rows once in the exhaustive fold case (#22358)
Part of #22346 Clause-②: no ## What changed One test case, test-only: `packages/lint/src/build-access-matrix.test.ts`, "exhaustively matches the fold over every declared bit combination". Its per-entry `m.entries.find(...)` lookup is replaced by one `Map`, built once from `m.entries` and keyed by `permissionSet`. The first match wins, so every lookup returns the same row `find` returned. The 3^8 enumeration, the cell count and all four assertions are unchanged. `build-access-matrix.ts` is untouched, no timeout is raised, and nothing is skipped. Why `Part of`: the card's generator closes the card when an hourly run is green. Its second cause, the shard-timing drift, belongs to #22014, which remains open. This PR removes only the lint case's half. ## Measurements All wall times were taken under `scripts/pm/os-verify-lock.sh` in a shared container. The lock excludes other locked runs, not unlocked sibling work, so the ratios carry more weight than the absolute numbers. **H1: the lookup is quadratic (confirmed).** A counter in the `find` callback, written the same way as the case, read 21,526,641 calls per run. That is 6,561 × 6,562 / 2, and each call rebuilds the `ps_NNNN` name. **H2: the lookup dominates (confirmed).** A scratch script (not committed) timed each step of the case three times at `54c3ce10c`: | step | iter 1 | iter 2 | iter 3 | |---|---|---|---| | enumerate the 3^8 entries | 4.5 ms | 1.0 ms | 0.9 ms | | map entries to permission sets | 1.6 ms | 1.9 ms | 0.6 ms | | `buildAccessMatrix` | 25.0 ms | 14.7 ms | 3.1 ms | | lookup, per-entry `find` (as on main) | 950.8 ms | 779.8 ms | 845.7 ms | | lookup, `Map` built once | 2.7 ms | 1.3 ms | 1.7 ms | | fold comparison loop | 5.6 ms | 1.7 ms | 1.7 ms | No other step in the case is quadratic. `buildAccessMatrix`, its sort included, costs too little to report. **Case wall time**, as vitest reports it per test. Command: `pnpm --filter @objectstack/lint exec vitest run --maxWorkers=2 --reporter=verbose src/build-access-matrix.test.ts`, three runs per tree: | tree | run 1 | run 2 | run 3 | |---|---|---|---| | before, `54c3ce10c` | 700 ms | 613 ms | 751 ms | | after, `b447bfa07` | 46 ms | 44 ms | 47 ms | That is about 15x faster. CI crossed the 5 s default only on a loaded shard (the triage measured a 473 s import phase there). At this container's speed, the case now runs at about 1/100 of the limit. **Ablation.** `scripts/ablation-replace.mjs` put the per-entry `find` back in place of the `Map` lookup three times, each in wrap mode. Each time the anchor count went 1 → 0, the replacement count went 0 → 1, and the blob went `84866b0a6a8f` → `40761cc2c49a`. | leg | run 1 | run 2 | run 3 | |---|---|---|---| | `find` put back (mutation) | 834 ms | 803 ms | 696 ms | All 8 cases passed under the mutation, which is expected: the timing is the pin, and no assertion was added. On every restore leg, the blob after restore equalled the blob at HEAD (`84866b0a6a8f`), and `git diff HEAD` was empty. ## The assertions are unchanged The case has 4 `expect(` calls before and after the change. Their matcher text is identical (compared as text with `cmp`): ``` expect(entries.length).toBe(3 ** 8) expect(m.entries.length).toBe(entries.length) expect(cells).toBe(3 ** 8 * 4) expect(mismatches).toEqual([]) ``` Across the whole file there are 28 `expect(` lines before and after, byte-identical as a sorted set, and 8 `it(` cases before and after. The full diff of the case: ```diff expect(m.entries.length).toBe(entries.length); + // Index the rows by permission set ONCE. A `find` per entry is quadratic — + // 3^8 entries make ~21.5M callbacks, each rebuilding the name — which ran + // this case past vitest's 5 s default on a loaded CI shard. First match + // wins, so every lookup returns the row `find` returned. + const rowByPermissionSet = new Map TYPE_ARGS (); + for (const e of m.entries) { + if (!rowByPermissionSet.has(e.permissionSet)) rowByPermissionSet.set(e.permissionSet, e); + } const mismatches: string[] = []; let cells = 0; for (let i = 0; i LESS_THAN entries.length; i++) { - const row = m.entries.find((e) => e.permissionSet === `ps_${String(i).padStart(4, '0')}`)!; + const row = rowByPermissionSet.get(`ps_${String(i).padStart(4, '0')}`)!; ``` Two placeholders stand in for angle-bracket text, which GitHub's body sanitizer removes. `TYPE_ARGS` is the Map's two type arguments: `string`, and the entry type written as `(typeof m.entries)[number]`. `LESS_THAN` is the less-than operator. The committed lines are in the file. ## Verification (at `b447bfa07`) - `pnpm --filter @objectstack/lint exec vitest run --maxWorkers=2`: 127 files passed; 5838 tests passed and 5 were skipped, out of 5843. - `pnpm --filter @objectstack/lint typecheck`: exit 0. - The edited file is in `tsconfig.test.json`'s program (`--listFiles`: 1 hit). The build `tsconfig.json` excludes it (0 hits). - `check:test-typecheck` passes, and this file is not on the debt ledger, so it compiles with zero errors. - ESLint, narrowed to the one changed file: `eslint --no-inline-config --format json packages/lint/src/build-access-matrix.test.ts` reports 1 file, 0 errors, 0 warnings. - Population: the diff's only path. `--print-config` shows that ESLint lints it, with 5 rules. - Invariance: the config enables no type-aware linting for this file (`parserOptions.project` and `projectService` are both null), and the diff edits no config. So no untouched file's verdict can change. - The repo-wide `pnpm lint` belongs to CI. - Gates: `node scripts/pm/dispatch-gates.mjs --commands` (no paths) at `b447bfa07` derives the same 54 commands as the list taken at dispatch. - 52 ran green on the first pass. - `check:lean-entry-closure` first refused with PREREQUISITE NOT MET. It is green after building the `@objectstack/objectql` closure. - `check:dual-build-cjs-loads` refused with PREREQUISITE NOT MET because it needs every package built. It is NOT MEASURED locally and is left to CI. - `--ran` verdict: "54 derived famil(ies) accounted for — 53 run, 1 NOT-MEASURED (1 DERIVED from a recorded exit 3)". ## Changeset No changeset; `skip-changeset` is applied instead. - The diff is test-only. Under AGENTS.md, the label is for a diff that publishes nothing from any released package. - Measured after `pnpm --filter @objectstack/lint build`: the package's `files` are `dist`, `README.md` and `CHANGELOG.md`. The new symbol `rowByPermissionSet` and the case title have 0 hits there; the positive control `buildAccessMatrix` has 6. - `check-empty-changeset.mjs` refuses an empty-frontmatter changeset, so the label is the declared opt-out. ## Acceptance notes - No out-of-scope findings. - The shard-timing drift (#22014) remains open and is not addressed here. This card stays open until an hourly run is green. --- _Generated by [Claude Code](https://claude.ai/code/session_01DhTqaEHqPVSVnAkjG3jywn)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 746637e commit b460153

1 file changed

Lines changed: 9 additions & 1 deletion

File tree

‎packages/lint/src/build-access-matrix.test.ts‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -167,10 +167,18 @@ describe('buildAccessMatrix CRUD columns — the spec fold, asked (#18785)', ()
167167
permissions: entries.map((e, i) => ({ name: `ps_${String(i).padStart(4, '0')}`, objects: { o: e } })),
168168
});
169169
expect(m.entries.length).toBe(entries.length);
170+
// Index the rows by permission set ONCE. A `find` per entry is quadratic —
171+
// 3^8 entries make ~21.5M callbacks, each rebuilding the name — which ran
172+
// this case past vitest's 5 s default on a loaded CI shard. First match
173+
// wins, so every lookup returns the row `find` returned.
174+
const rowByPermissionSet = new Map<string, (typeof m.entries)[number]>();
175+
for (const e of m.entries) {
176+
if (!rowByPermissionSet.has(e.permissionSet)) rowByPermissionSet.set(e.permissionSet, e);
177+
}
170178
const mismatches: string[] = [];
171179
let cells = 0;
172180
for (let i = 0; i < entries.length; i++) {
173-
const row = m.entries.find((e) => e.permissionSet === `ps_${String(i).padStart(4, '0')}`)!;
181+
const row = rowByPermissionSet.get(`ps_${String(i).padStart(4, '0')}`)!;
174182
for (const verb of ['create', 'read', 'edit', 'delete'] as const) {
175183
cells++;
176184
if (row[verb] !== referenceFold(entries[i], verb)) {

0 commit comments

Comments
 (0)