test(core): the HTML scanner's linear-work checks count work instead of timing it - #5037
Conversation
Edit accuracy: accurate 2040 (base branch 2040), smooth 1656 of thoseThe gate passes. Quarantined, measured but not gated (0) |
jrusso1020
left a comment
There was a problem hiding this comment.
Reviewed at dce56949. Approving.
What I checked:
- The counter measures the real scanner.
scannedCharspatchesString.prototypeandRegExp.prototype.execthemselves, so the calls it counts arehtmlDocument.ts's own:lowerAscii'sreplaceandexec,markupStarts'sindexOf,findTagEnd'scharAt,isTagAt'sstartsWith, andskipComment's stickyCOMMENT_END.exec. Patchingexecon the prototype also routestestand the regexreplacethrough it. The originals are restored infinally. - Every case measures real work. I logged the counts. At n = 10,000 each case counts 50,023 to 560,076 characters, at least 5 times n, and every ratio is 2.00 (1.998 to 2.001). No case is linear just because it counts close to nothing.
- Inputs are built outside the count.
repeatand template literals aren't patched. The raw-text case's two correctness checks moved to n = 3 and still compare the whole output.
Tests I ran:
htmlDocument.test.ts: 51 of 51, three runs in a row, about 0.11 s of test time each.- Mutants I added, beyond the two in the PR body:
isTagAtwritten aslowered.indexOf(token, at) === at, a realistic O(n²) slip: 4 cases fail (raw-text, many <, many , many comments).- Re-lowercasing the document for every tag in
findDocumentTag: the same 4 fail. - Ignoring the
unclosedRawTextmemo inskipRawText: the raw-text case fails. - A quadratic loop hidden in bracket reads in
findTagEnd: 12 of 13 adversarial cases stay green, and the raw-text case fails only by running 292 s. That is the blind spot your body names. It confirms the counter depends on the scanner keeping its reads to the patched methods.
Reuse: the same stopwatch is one file over, on the same scanner. scenePartsManifest.test.ts:67-78 times addScenePartsManifest (which calls injectTagsAtHeadStart) on 40,000 <head against 1000 ms, which is the same flake on a loaded runner. browserManager.test.ts:223-235 (500 ms) and lint/sourceLocations.test.ts:205-211 (1000 ms) have the same shape. Moving scannedChars and workGrowth into a shared test helper and converting at least the scenePartsManifest case would close the flake for that scanner. That fits a follow-up, so I'm not blocking on it.
Simpler? Hardly. Counting characters examined is the smallest thing that is independent of load. The only trim is includes, which nothing in htmlDocument.ts calls.
Verdict: APPROVE
Reasoning: The counter instruments the scanner's own string and regex calls. Every case counts real, linear work at a ratio of 2.00, and three realistic quadratic slips each fail their cases. The one blind spot is documented in the PR body, and I confirmed it.
— Rames Jusso
What
The HTML scanner's "stays linear" tests in
core/src/compiler/htmlDocument.test.tsno longer use a stopwatch. They count the work the scanner does instead, so they give the same answer on every machine and under any load, and still fail if the scanner turns quadratic.Why
The 13 "stays fast on …" cases timed
injectTagsAtHeadStarton a 2 MB adversarial input against a fixed 1500 ms budget, and the raw-text case against 500 ms. The scanner itself is linear and fast (about 0.1 µs per<on an idle machine), but on a busy machine (load average around 20, parallel test workers) the same input took 2 to 3.7 s and the case failed, then passed when run alone. A fixed wall-clock budget cannot tell a slow machine from a slow algorithm.How
scannedChars(run)wraps the string methods that scan text (indexOf,lastIndexOf,includes,startsWith,charAt,replace,toLowerCase) andRegExp.prototype.exec(whichtest,replaceandmatchgo through) for the duration of the call, adds up the characters each call examines, then restores them. It watches only those methods andexec: text read any other way (bracket reads,at,charCodeAt,split,for..of, spreads) is not counted, so a future rewrite of the scanner'scharAtreads to one of those would blind the check; the scanner uses none of them on these paths today. A sticky regex that misses is charged to the end of its input, which can only make the check stricter, never let a quadratic case through.workGrowth(scan)compares the work at n = 20,000 with the work at n = 10,000. Linear work gives about 2; quadratic work gives about 4. Each case expects less than 2.5.No timers remain in the file, and the 13 adversarial inputs are 100 times smaller, so the file runs in about 0.1 s instead of about 0.9 s.
Test plan
htmlDocument.test.ts3 runs in a row, all green, numbers identical each run.markupStarts(a realistic accidental O(n²)) fails the 4 cases that exercise that path at growth 4.0; searching for a comment's end from the start of the document every time fails "many comments" at 3.98. Taking a string slice inside the loop, which V8 does without copying, stays green (no false alarm).