-
Notifications
You must be signed in to change notification settings - Fork 3k
fix(web-shell): cap React dev performance.measure accumulation to stop renderer OOM #9770
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+200
−6
Merged
Changes from all commits
Commits
Show all changes
4 commits
Select commit
Hold shift + click to select a range
8c2c638
fix(web-shell): cap React dev performance.measure accumulation
wenshao 0cf30f7
test(web-shell): pin the measure-guard contract against mutations
wenshao e8219d6
test(web-shell): close the measure-guard mutation surface systematically
wenshao 66864a8
test(web-shell): pin non-React measure detail survival by value
qwen-code-dev-bot File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[Suggestion] R2-1: The measure-guard test's mutant surface is still not closed — this round's probe verified another survivor under the committed suite, so the round-3 commit's closure claim ("33/33 mutants killed; remaining mutants are genuinely equivalent") has not reached its own stated fixpoint ("iterated until no mutant survives"). The demonstrated entrance: a mutant that strips a non-React measure's
detailby mutating the caller's options object in place (e.g.options.detail = nullbefore forwarding — the allocation-free variant of the existing strip, plausible under the HTML comment's own "~16k measures/s" framing) passes the entire suite, because the "not stripped" half of this contract is pinned only by identity comparisons —toBe(customOptions)still passes on the same object once mutated, and the earliertoHaveBeenCalledWith('custom-measure', options)compares the recorded argument against that same mutated reference (self-equality). A future guard edit shaped like that would silently corrupt a caller-owned options object (web-shell code reusing one options object across measures loses itsdetail) while this comment claims the case is pinned. The React path IS protected against the same mutant shape (the first test readsoptions.detail.devtools.propertiesafter the call); the non-React path has no equivalent post-call value read. Witness (probe in a scratch tree at this commit): the mutant survives with the suite green (7/7); adding the value-survival assertion below flips it red (expected null to deeply equal { source: 'web-shell' }); the same in-place strip on the React branch fails the existing post-call read (Cannot read properties of null (reading 'devtools')); and the correct guard with the proposed assertion stays green (7/7). Fix: fold this mutant into the mutation pass, re-run it to fixpoint, and keep the killing assertion below, which closes the demonstrated entrance.中文说明
[Suggestion] measure-guard 测试的变异面仍未收口——本轮探针在已提交的套件下又验证了一个幸存变异体,因此第 3 轮提交的收口声明("33/33 变异体全部被杀死;其余变异体均为等价变异")并未达到其自身设定的收敛点("迭代到没有变异体幸存")。已验证的入口:在转发非 React measure 之前,通过原地修改调用方 options 对象来剥离
detail的变异体(例如转发前options.detail = null——现有剥离的零分配变体,在 HTML 注释自身 "~16k measures/s" 的语境下相当合理)能通过整套测试,因为该契约中"未被剥离"这半边仅由引用相等断言钉住——对象被原地修改后toBe(customOptions)仍然通过,而前面的toHaveBeenCalledWith('custom-measure', options)是 recorded 参数对同一被修改引用的自比较。未来若 guard 按此形态修改,将悄悄破坏调用方自有的 options 对象(复用同一 options 对象的 web-shell 代码会丢失其detail),而此处注释却声称该情形已被钉住。React 路径对同形变异体有防护(第一个测试在调用后读取options.detail.devtools.properties);非 React 路径缺少等价的调用后取值断言。见证(在本提交的 scratch tree 中探针验证):变异体幸存、套件保持绿色(7/7);补充下方的取值存活断言后由绿变红(expected null to deeply equal { source: 'web-shell' });同形变异体作用于 React 分支时会被现有的调用后读取杀死(Cannot read properties of null (reading 'devtools'));正确 guard 加上该断言保持绿色(7/7)。修复:将该变异体并入变异测试通道,重新迭代至无幸存者,并保留下方断言——它可杀死已验证的这个入口。— qwen3.8-max via Qwen Code /review (v0.22.0)