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
[JSC] DFG AI should attempt to remove @tryGetById for String.replace #4522
[JSC] DFG AI should attempt to remove @tryGetById for String.replace #4522
Conversation
EWS run on previous version of this PR (hash 21367ae) |
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.
Seems okay, but...
- Ensure RegExp.prototype's properties via watchpoints.
- Check structure in AI, and ensure that this structure is one for RegExpObject, its [[Prototype]] is RegExp.prototype, it is cacheable,
and it does not have properties for particular names.
...(1) appears to be existing functionality and not actually part of this patch?
@@ -3424,12 +3425,56 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi | |||
makeHeapTopForNode(node); | |||
break; | |||
|
|||
case TryGetById: | |||
case TryGetById: { | |||
// This is very adhoc, but @tryGetById is not usually used in user code, and it is used adhocly in very limited places. |
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.
Seems like you don't need the "usually"?
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.
Right, dropped.
Yes |
21367ae
to
cc558c3
Compare
https://bugs.webkit.org/show_bug.cgi?id=245418 Reviewed by Ross Kirsling. @tryGetById is adhoc one: it is used only in builtin code and used in very limited places. So this patch introduces adhoc folding rules for @tryGetById to remove TryGetById attached before String.replace. Unfortunately these @tryGetById are not usually removed since it is adhocly attached in DFG fixup phase. In this patch, 1. Ensure RegExp.prototype's properties via watchpoints. 2. Check structure in AI, and ensure that this structure is one for RegExpObject, its [[Prototype]] is RegExp.prototype, it is cacheable, and it does not have properties for particular names. Then we fold TryGetById to RegExp's specific properties values. It eliminates these TryGetById and improves String.replace in DFG / FTL by 65% (e.g. string-replace). ToT Patched string-replace-generic 33.0472+-0.1132 ^ 31.9405+-0.1483 ^ definitely 1.0346x faster string-replace 3.3774+-0.0951 ^ 2.0387+-0.0476 ^ definitely 1.6566x faster put-by-val-with-string-replace-and-transition 4.4385+-0.0233 4.4319+-0.0188 string-replace-benchmark 83.8759+-0.1186 ^ 83.4655+-0.2393 ^ definitely 1.0049x faster string-replace-string 380.3462+-0.7600 379.2646+-1.0203 string-replace-empty 3.2455+-0.0406 ^ 2.0051+-0.1350 ^ definitely 1.6186x faster * Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h: (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects): Canonical link: https://commits.webkit.org/254717@main
Committed 254717@main (e289c32): https://commits.webkit.org/254717@main Reviewed commits have been landed. Closing PR #4522 and removing active labels. |
cc558c3
to
e289c32
Compare
e289c32
cc558c3
π macπ wpeπ π§ͺ winπ mac-debugπ wincairoπ§ͺ ios-wk2π mac-AS-debugπ§ͺ gtk-wk2π§ͺ api-iosπ§ͺ api-macπ§ͺ api-gtkπ tvπ§ͺ mac-wk1π tv-simπ§ͺ mac-wk2π watchπ§ͺ mac-AS-debug-wk2π watch-simπ§ͺ mac-wk2-stress