mirror of
https://github.com/civitai/civitai.git
synced 2026-09-20 22:08:18 +08:00
Justin Maier
7048aadfa1
* perf(tests): route sharp-executing tests to their own forks project sharp 0.32.6's addon is not context-aware, so a worker_threads worker that has run a libvips operation segfaults at thread teardown - after the tests pass and the summary prints. That takes the whole run down with an exit code and no failing test. It is a race, not a threshold. On the six affected files, three repeats per width: vmThreads crashed 3/3 at 1 worker, 2/3 at 2, 1/3 at 3, 0/3 at 4; threads crashed at every width. A green run is not evidence of safety. Importing sharp is harmless; only executing an operation arms it. 100 test files carry sharp in their static closure and exactly six call it. That set was measured by aliasing sharp to a recording proxy and running all 100 under forks, not by grepping and not by a crash-scan - with a race, "ran alone and didn't crash" builds the list out of the files that got lucky. - unit -> the suite minus those six, pool unchanged (forks) - unit-native -> pool: forks pinned, including only those six unit deliberately does NOT move to threads. threads measured 1.04x at 4 workers and 0.94x at 16 - no win - and it segfaults mid-run on the full suite at roughly 1 in 4, after completing hundreds of files cleanly and with no unit-native file having run, so a second crasher exists that is not sharp and is not diagnosed. What the split buys is that the sharp crash is deterministic and gone, so anyone experimenting with --pool=threads no longer has to fight it too. Excluded from unit rather than merely claimed by unit-native, so that a run naming a sharp file under --project unit reports "No test files found" rather than running it on a thread pool if one is ever selected. Shared settings are hoisted into one object both projects spread, so they cannot drift apart. Every selector moves to a unit* project pattern. Verified by file count rather than by a green summary: vitest list over unit* is 1065 files and unit-native is 6, summing to the pre-split baseline. no-sharp-outside-native-project.test.ts is the positive control, since nothing else in the suite can notice this breaking: the realistic failure is a rename, which stops matching unit-native's include AND unit's exclude. Mutation-tested, not assumed green. * perf(tests): pre-bundle five externals nothing mocks Every test file gets a fresh module registry - under forks with isolate: true it is literally a fresh child process per file (N files at maxWorkers=1 give N distinct pids), so Node's module cache dies with it and each externalised package is imported cold once per file that reaches it. Pre-bundling collapses a package's many-hundred-file native load into one chunk, paid once per run. Full suite, control then treatment in one window: control wall 217.2s collect 4730s 1066 files 16787 tests 17 failed treatment wall 192.9s collect 4082s 1066 files 16787 tests 17 failed The failure SET is unchanged, diffed both directions - nothing appeared, nothing cleared. This alters timing, not behaviour. The effect tracks exposure, which is what separates it from ambient drift: reaches 0 of the 5 353 files collect 113s -> 105s -6.9% reaches 1 115 files collect 150s -> 103s -31.2% reaches 2 209 files collect 529s -> 373s -29.6% reaches 3 18 files collect 69s -> 52s -24.3% reaches 5 371 files collect 3869s -> 3449s -10.9% Exposure 5 shows the smallest percentage and the largest absolute saving (420s of 648s) because those are the heavyweight files: five packages are a small share of a 1,300-module closure and a large share of a small one. Percentage tracks share-of-closure, absolute tracks file weight. A control-vs-control run to size run-to-run drift directly was attempted and died to an unrelated crash, so the residual drift term is unmeasured and the figure above should be read as an upper bound. The list is confined to packages nothing mocks, and that is load-bearing. Pre-bundling wraps a package as a CJS-interop chunk, so a vi.mock factory returning only named exports stops satisfying its consumers - adding redis and the mocked aws-sdk clients takes four mock-holding files from 92 tests passing to 7 collected. The importOriginal form does not protect against this. Those three are worth ~275s more and need a default export added to six mock factories first; that is a separate change. The treatment paid its cold optimize pass inside the measured run - the shared .vite cache was not cleared - so the number is not flattered by a warm cache, and a fresh CI runner pays the same thing. * test(perf): pin the native project's pool independently of unit's The guard asserted unit ran on threads, which was the state the split shipped in for about ten minutes. It caught its own config change, which is the behaviour wanted, but the assertion was aimed at the wrong invariant: what must hold is that unit-native stays on a process-based pool whatever unit is pointed at, not that unit is on any particular one. * test(perf): acceptance harness for the six external-mock factories The change that brings redis and the aws-sdk clients into the pre-bundling safelist cannot be verified on a tree that does not enable the optimizer: without pre-bundling the package is not wrapped as a CJS-interop chunk, the missing default export never bites, and a green run proves only that the old config still works. This runs the six affected files under a config with all three candidates pre-bundled. Compares per-file collected counts rather than the total. s3-utils is 66 of the 106, so a sum of 40 could be one file collecting zero and still read as a partial pass. Negative control on the unchanged tree: 5 of 6 files collect 0, and the harness exits 1 naming each one. * docs(test-perf): record the measurement envelope this box imposes Two identical full runs, back to back, nothing changed between them, came out +20.5% apart on collect. That pair was contaminated, so it is not a drift figure - but it demonstrates the box can move further than most of the effects measured today, which means any comparison assembled from two windows is unreadable. Collects the methodology that follows: quote in-pair controls rather than cross-window deltas; a control group must be comparable in cost and not merely in count; a dose-response on an axis confounded with file cost is suggestive rather than conclusive; a crashed run's wall clock is not a fast run; and check for the workload rather than for the runtime when deciding the box is quiet. A clean drift pair still has not been taken and is the denominator for everything else here. * docs(test-perf): scout Bun and node:test as vitest replacements Recommendation is to stay on vitest, but the measurement overturns the cost model we spent the day optimising against. Same 84-module first-party graph: vitest collect 5298ms, bun 3.3ms, node+tsx 8.5ms. Whole-suite arithmetic gives vitest 10.2ms per static module-instance against 0.04ms for bun. The cost is the module runner, not the modules - which matches this morning's tracer result (569 module bodies in ~0.4s against a 25.4s import phase) and locates the time in vite-node's per-module fetch/instantiate rather than in compile-and-evaluate. Unrealisable, though. Bun cannot load any graph reaching the React/Next side - it dies resolving use-sidecar's package exports - so the numbers are measured on the light stratum only, which is the flattering-slice trap: 383 of 1065 files have no infra dependency and the largest of those is an 86-module closure. Module-scope env aborts the import under both runtimes, and cache-helpers hung past 300s under bun after the env gate was satisfied. The mock surface is the wall: 1053 of 1065 files import from vitest, 3883 vi.mock sites across 651 files, plus 8053 vi.fn and the fake-timer, spy and importActual surface. The canonical mock system, its guard, the allowlist ratchet, reporter.mjs, the dashboard and the queue integration are all vitest-shaped as well. Retarget rather than switch: if per-module cost is vite-node overhead, shrinking the graph attacks a term worth ~0.04ms of real work per module, and the leverage is in how many times a module is INSTANTIATED - which is what isolate:false removes. * docs(test-perf): retract the per-module ratio in the runner scouting It divided collect by inventory.json's static module counts, and that artifact was wrong by up to 75x and selectively so - it followed lazy dynamic import edges that never execute and ignored vi.mock factories. Honest suite union is 1321, not 3230. The per-file wall clock the recommendation rests on needs no denominator and is unaffected: same 84-module closure, vitest collect 5298ms against bun 3.3ms and node+tsx 8.5ms. So is the tracer result behind it - 569 module bodies executing in ~0.4s against a 25.4s import phase, measured with no static count at all. No counterpart figure is quoted for bun, because its denominator came from the same artifact. * docs(test-perf): scrub the remaining per-module claims from the runner scout Two survivors of the retraction: an 'orders of magnitude per module' headline and a stratum characterisation quoting closure sizes, both resting on the same broken counts. Restated against the per-file wall clock, which needs no denominator, and the observed hard failure, which is not a count. * docs(test-perf): correct the runner comparison to like-for-like The headline compared vitest's collect for a TEST FILE against a probe importing only the SOURCE module underneath it - a different and much smaller graph. That is where '~1600x' came from. Like-for-like, on the same 82-module test-file closure: vitest collect 5298ms, bun 259ms (median of 5, 250-262). ~20x, not three orders of magnitude. node+tsx cannot import a test file at all - 'Vitest cannot be imported in a CommonJS module using require()'. Per-module refit against aidan's honest closures.json (mode: 'real') joined to the pre-ctl full run: 1065 files, 104797 real module-instances, collect 4729s -> vitest 45.1 ms/module, independently agreeing with aidan's 43.6. bun 3.2 ms/module on the file both can load. The recommendation is unchanged and the mechanism finding is unchanged; the size of the gap was overstated. * docs(vitest): say why the unit projects set no per-project maxWorkers Per-project maxWorkers does apply at runtime, but two projects with different counts need different sequence.groupOrder values, and different groups run serially. For a 1059/6 split that trades the concurrency between them for a knob nobody needs - the six-file project would gate the other 1059 instead of filling spare capacity beside it. Currently reads as an omission, so a future reader adds one and loses concurrency without knowing they traded for it. * docs(test-perf): final form of the runner scouting result Leads with both corrections stated in place rather than silently edited out, and records that neither changed the recommendation. Promotes the cross-validation to a finding of its own: 45.1 ms per module-instance here against aidan's independent 43.6, from a different artifact by a different route. Two wrong denominators would not have agreed, so the pair is what licenses everything downstream that divides by a module count. Names what both errors had in common - each a denominator error producing a number right about the thing it measured and wrong about what that thing was. Checking two runtimes are comparable is not checking the two quantities are. * fix(test): pin unit-native's pool against a CLI --pool, and correct the project selector `unit-native`'s static `pool: 'forks'` loses to a CLI `--pool=threads`: resolveProjects builds cliOverrides from a list that includes `pool` and spreads it after options.test, so the flag wins. The six sharp-executing files would then follow `unit` onto a thread pool and segfault AFTER printing a green summary. configureVitest hooks run after resolveProjects(cliOptions), and getFilePoolName -- `browser.enabled ? 'browser' : project.config.pool` -- is what stamps each spec's pool, so re-asserting there outranks the flag. The other two readers of project.config.pool populate task metadata from the same field and cannot disagree with it. The comment already claimed this guarantee; without the plugin it was false, and false in the reassuring direction. Also corrects CLAUDE.md: the unit suite is two projects now, so `--project unit` silently runs 1059 of 1065 files and exits 0. Select it as `--project 'unit*'`, which is what package.json's own scripts already do. Bound: the pin covers `pool` and nothing else. isolate, fileParallelism, sequence, testTimeout and retry are on the same cliOverrides list and remain overridable. Not verified by a run -- the mechanism was read from vitest 4.0.18's cli-api chunk twice, independently, by two readers. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UtTG4QQR29eWf7kjM6HiLU --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
29 lines
1.1 KiB
TypeScript
29 lines
1.1 KiB
TypeScript
/**
|
|
* The root config with `sharp` aliased to scripts/test-perf/sharp-usage-probe.mjs, so a run
|
|
* records which test files execute a libvips operation. Runs under `forks`, which survives.
|
|
*
|
|
* Derived from the real config rather than restating it — the `unit` project's alias list is
|
|
* load-bearing (nine workspace packages that fail to COLLECT without it), and a hand-copied
|
|
* version would rot silently.
|
|
*/
|
|
import path from 'path';
|
|
import base from '../../vitest.config.mts';
|
|
|
|
const sharpAlias = {
|
|
find: /^sharp$/,
|
|
replacement: path.resolve(import.meta.dirname, 'sharp-usage-probe.mjs'),
|
|
};
|
|
|
|
const config = base as any;
|
|
const projects = config.test.projects.filter(
|
|
(p: any) => typeof p === 'object' && p.test?.name === 'unit'
|
|
);
|
|
if (projects.length !== 1) {
|
|
throw new Error(`expected exactly one 'unit' project, found ${projects.length}`);
|
|
}
|
|
// Must precede the `~` entry so it wins, same ordering rule the componentAlias follows.
|
|
projects[0].resolve.alias = [sharpAlias, ...projects[0].resolve.alias];
|
|
config.test.projects = projects;
|
|
|
|
export default config;
|