Bug 21294 - Devirtualize getOwnPropertySlot()
: Devirtualize getOwnPropertySlot()
Status: RESOLVED FIXED
: WebKit
JavaScriptCore
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
:
: 20813
  Show dependency treegraph
 
Reported: 2008-10-01 23:01 PST by
Modified: 2008-10-22 17:16 PST (History)


Attachments
patch, working but ChangeLog not done yet (52.44 KB, patch)
2008-10-22 10:08 PST, Darin Adler
no flags Review Patch | Details | Formatted Diff | Diff
patch (50.40 KB, patch)
2008-10-22 12:00 PST, Darin Adler
sam: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2008-10-01 23:01:21 PST
We always use a virtual call for getOwnPropertySlot(), when we should really be able to inline the common cases in many of its callers, or at least use non-virtual calls. This could be achieved by a bit in the StructureID TypeInfo, just like was done for 'instanceof'.
------- Comment #1 From 2008-10-16 22:02:38 PST -------
Sam is now working on other tasks, so I will assign this to myself.
------- Comment #2 From 2008-10-22 09:39:10 PST -------
*** Bug 21793 has been marked as a duplicate of this bug. ***
------- Comment #3 From 2008-10-22 10:08:51 PST -------
Created an attachment (id=24559) [details]
patch, working but ChangeLog not done yet
------- Comment #4 From 2008-10-22 12:00:55 PST -------
Created an attachment (id=24561) [details]
patch
------- Comment #5 From 2008-10-22 13:08:24 PST -------
(From update of attachment 24561 [details])
-    // WebCore uses this to make document.all and style.filter undetectable.
     static const unsigned MasqueradesAsUndefined = 1;
+        // WebCore uses MasqueradesAsUndefined to make document.all and style.filter undetectable.
     static const unsigned ImplementsHasInstance = 1 << 1;
The whitespace for this comment seems wrong.

+JSGlueGlobalObject::JSGlueGlobalObject(PassRefPtr<StructureID> structure, JSFlags flags)
+    : JSGlobalObject(structure, new Data, this)
+{
+    d()->flags = flags;
+    d()->userObjectStructure = UserObjectImp::createStructureID(jsNull());
+}
Might be cleaner to make the Data constructor take arguments.

+    if (prototype->isNull()) {
+        static StructureID* structureID = JSInspectorCallbackWrapper::createStructureID(jsNull()).releaseRef();
+        return new (unwrappedExec) JSInspectorCallbackWrapper(unwrappedExec, unwrappedObject, structureID);
+    }
Why is leaking the StructureID okay here?

For QtRuntimeMethod::createStructureID, why does it have the ImplementsHasInstance flag like InternalFunction?

r=me. 
------- Comment #6 From 2008-10-22 16:25:30 PST -------
(In reply to comment #5)
> (From update of attachment 24561 [details] [edit])
> -    // WebCore uses this to make document.all and style.filter undetectable.
>      static const unsigned MasqueradesAsUndefined = 1;
> +        // WebCore uses MasqueradesAsUndefined to make document.all and
> style.filter undetectable.
>      static const unsigned ImplementsHasInstance = 1 << 1;
> The whitespace for this comment seems wrong.

I'll move the comment back.

> +    d()->flags = flags;
>
> Might be cleaner to make the Data constructor take arguments.

Sure, but since this is just JavaScriptGlue, I think I'll leave it as-is.

> +    if (prototype->isNull()) {
> +        static StructureID* structureID =
> JSInspectorCallbackWrapper::createStructureID(jsNull()).releaseRef();
> +        return new (unwrappedExec) JSInspectorCallbackWrapper(unwrappedExec,
> unwrappedObject, structureID);
> +    }
>
> Why is leaking the StructureID okay here?

As we discussed, I'll use StructureID::startIgnoringLeaks().

> For QtRuntimeMethod::createStructureID, why does it have the
> ImplementsHasInstance flag like InternalFunction?

Oops, will fix.
------- Comment #7 From 2008-10-22 17:16:23 PST -------
http://trac.webkit.org/changeset/37799