Bug 132721 - JSDOMWindow should have a WatchpointSet to fire on window close
Summary: JSDOMWindow should have a WatchpointSet to fire on window close
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Hahnenberg
URL:
Keywords:
Depends on:
Blocks: 132705
  Show dependency treegraph
 
Reported: 2014-05-08 18:00 PDT by Mark Hahnenberg
Modified: 2014-05-09 13:26 PDT (History)
0 users

See Also:


Attachments
Patch (11.10 KB, patch)
2014-05-08 18:24 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Patch (11.11 KB, patch)
2014-05-08 19:09 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Patch (13.79 KB, patch)
2014-05-09 11:49 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Patch (13.83 KB, patch)
2014-05-09 12:19 PDT, Mark Hahnenberg
fpizlo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Hahnenberg 2014-05-08 18:00:41 PDT
This would allow us to reset the inline caches that assumed that they could skip the first part of JSDOMWindow::getOwnPropertySlot.
Comment 1 Mark Hahnenberg 2014-05-08 18:24:15 PDT
Created attachment 231119 [details]
Patch
Comment 2 Mark Hahnenberg 2014-05-08 19:06:23 PDT
Comment on attachment 231119 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=231119&action=review

> Source/WebCore/bindings/js/JSDOMWindowBase.cpp:280
> +            return;

continue;

> Source/WebCore/bindings/js/JSDOMWindowBase.cpp:283
> +            return;

continue;
Comment 3 Mark Hahnenberg 2014-05-08 19:09:12 PDT
Created attachment 231121 [details]
Patch
Comment 4 Geoffrey Garen 2014-05-09 00:03:24 PDT
Comment on attachment 231121 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=231121&action=review

r=me

> Source/JavaScriptCore/jit/Repatch.cpp:334
> +    if (watchpointSet)
> +        watchpointSet->add(stubInfo.addWatchpoint(codeBlock));
> +

Is this really the only place we'll ever specialize a window object property access? Is there some kind of struct flag to guarantee that? For example, what prevents the DFG from fully inlining the property access?

> Source/WebCore/bindings/js/JSDOMWindowBase.cpp:275
> +    clientData->getAllWorlds(wrapperWorlds);

If DOMWindow were ScriptWrappable, you would need a special case to clear its inline wrapper. But it isn't. So yay!
Comment 5 Mark Hahnenberg 2014-05-09 11:22:45 PDT
(In reply to comment #4)
> (From update of attachment 231121 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=231121&action=review
> 
> r=me
> 
> > Source/JavaScriptCore/jit/Repatch.cpp:334
> > +    if (watchpointSet)
> > +        watchpointSet->add(stubInfo.addWatchpoint(codeBlock));
> > +
> 
> Is this really the only place we'll ever specialize a window object property access? Is there some kind of struct flag to guarantee that? For example, what prevents the DFG from fully inlining the property access?

Now that you mention it, the DFG definitely needs to know about this. This will require more infrastructure, so I'll post a new patch.
Comment 6 Mark Hahnenberg 2014-05-09 11:49:35 PDT
Created attachment 231172 [details]
Patch
Comment 7 Mark Hahnenberg 2014-05-09 12:19:08 PDT
Created attachment 231177 [details]
Patch
Comment 8 Filip Pizlo 2014-05-09 12:52:57 PDT
Comment on attachment 231177 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=231177&action=review

> Source/JavaScriptCore/bytecode/PolymorphicGetByIdList.h:84
>      bool doesCalls() const { return type() == Getter || type() == CustomGetter; }
> +    bool isWatched() const { return type() == WatchedStub; }

Add an isSimple() and assert that instead of the places where we currently assert !doesCalls().
Comment 9 Mark Hahnenberg 2014-05-09 13:26:48 PDT
Committed r168548: <http://trac.webkit.org/changeset/168548>