Bug 237503 - Optimize VMTraps::maybeNeedHandling().
Summary: Optimize VMTraps::maybeNeedHandling().
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Lam
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2022-03-05 11:22 PST by Mark Lam
Modified: 2022-03-05 13:57 PST (History)
6 users (show)

See Also:


Attachments
proposed patch. (3.32 KB, patch)
2022-03-05 11:29 PST, Mark Lam
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Lam 2022-03-05 11:22:27 PST
There's no need for VMTraps::maybeNeedHandling() to mask the VMTraps bits for events.  Under normal circumstances, there are no traps firing and the traps bits are 0 anyway.  We should optimize for this and do away with the masking.  Clients who use VMTraps::maybeNeedHandling() should and current does call VMTraps::needHandling() to get the real story on whether there are actually traps to handle or not.  Hence, the masking in VMTraps::maybeNeedHandling() is also not needed for correctness.
Comment 1 Mark Lam 2022-03-05 11:29:55 PST
Created attachment 453916 [details]
proposed patch.
Comment 2 Saam Barati 2022-03-05 12:58:01 PST
Comment on attachment 453916 [details]
proposed patch.

Going back to r?. They’re non zero when we defer, which may happen frequently?
Comment 3 Saam Barati 2022-03-05 12:58:33 PST
(In reply to Saam Barati from comment #2)
> Comment on attachment 453916 [details]
> proposed patch.
> 
> Going back to r?. They’re non zero when we defer, which may happen
> frequently?

Maybe not frequently enough though. Might be worth just giving a bit of thought to defer% to non-defer%
Comment 4 Mark Lam 2022-03-05 13:00:38 PST
(In reply to Saam Barati from comment #3)
> (In reply to Saam Barati from comment #2)
> > Comment on attachment 453916 [details]
> > proposed patch.
> > 
> > Going back to r?. They’re non zero when we defer, which may happen
> > frequently?
> 
> Maybe not frequently enough though. Might be worth just giving a bit of
> thought to defer% to non-defer%

I think that would be rare in a few sites only.  We shouldn't be penalizing all exception check sites to favor just a few defer sites.

Plus if bots show a regression, we can just roll this back.
Comment 5 Saam Barati 2022-03-05 13:08:16 PST
Comment on attachment 453916 [details]
proposed patch.

r=me
Comment 6 Mark Lam 2022-03-05 13:11:27 PST
Comment on attachment 453916 [details]
proposed patch.

Thanks for the review.
Comment 7 EWS 2022-03-05 13:56:47 PST
Committed r290871 (248102@main): <https://commits.webkit.org/248102@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 453916 [details].
Comment 8 Radar WebKit Bug Importer 2022-03-05 13:57:17 PST
<rdar://problem/89861322>