Skip to content

jsc: make control-flow-profiler getExecutedRanges linear instead of quadratic - #36137

Open
robobun wants to merge 8 commits into
mainfrom
farm/73bfe023/jsc-coverage-getexecutedranges
Open

jsc: make control-flow-profiler getExecutedRanges linear instead of quadratic#36137
robobun wants to merge 8 commits into
mainfrom
farm/73bfe023/jsc-coverage-getexecutedranges

test: replace vacuous allValid assertion with range-ordering check

e5594f4
Select commit
Loading
Failed to load commit list.
Claude / Claude Code Review completed Jul 27, 2026 in 13m 29s

Code review found 1 potential issue

Found 3 candidates, confirmed 1. See review comments for details.

Details

Severity Count
🔴 Important 0
🟡 Nit 1
🟣 Pre-existing 0
Severity File:Line Issue
🟡 Nit test/js/node/inspector/inspector-profiler.test.ts:776 Replacement sorted assertion is also vacuous — buildScriptCoverageList re-sorts blocks

Annotations

Check warning on line 776 in test/js/node/inspector/inspector-profiler.test.ts

See this annotation in the file changed.

@claude claude / Claude Code Review

Replacement `sorted` assertion is also vacuous — buildScriptCoverageList re-sorts blocks

The `sorted` check that replaced `allValid` in e5594f4 is unfortunately just as vacuous — `buildScriptCoverageList` chains `.sort((a, b) => a[0] - b[0])` right after the `.filter()` on inspector.ts:309, so `ranges[1..]` is ascending by `startOffset` no matter what order JSC emits. My earlier suggestion missed that the sort sits on the same line as the filter it cited; sorry for the misdirection. Only the `topLevelRanges >= N` check is load-bearing here — I'd just drop `sorted` from the fixture o