Bug 68937 - De-virtualize JSCell::toObject
Summary: De-virtualize JSCell::toObject
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: 67690
  Show dependency treegraph
 
Reported: 2011-09-27 14:32 PDT by Mark Hahnenberg
Modified: 2011-09-29 19:08 PDT (History)
2 users (show)

See Also:


Attachments
Patch (5.85 KB, patch)
2011-09-27 14:40 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Patch (6.94 KB, patch)
2011-09-28 11:32 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Patch (6.94 KB, patch)
2011-09-28 11:58 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Patch (8.72 KB, patch)
2011-09-29 14:49 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Benchmark results (without inlining) (6.41 KB, text/plain)
2011-09-29 16:34 PDT, Mark Hahnenberg
no flags Details
Benchmark results (with inlining) (6.35 KB, text/plain)
2011-09-29 16:34 PDT, Mark Hahnenberg
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Hahnenberg 2011-09-27 14:32:37 PDT
Part of process of de-virtualizing JSCell.
Comment 1 Mark Hahnenberg 2011-09-27 14:40:49 PDT
Created attachment 108902 [details]
Patch
Comment 2 Mark Hahnenberg 2011-09-28 11:32:54 PDT
Created attachment 109041 [details]
Patch
Comment 3 Mark Hahnenberg 2011-09-28 11:58:01 PDT
Created attachment 109044 [details]
Patch
Comment 4 Geoffrey Garen 2011-09-29 12:00:03 PDT
Comment on attachment 109044 [details]
Patch

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

r=me with the change below:

> Source/JavaScriptCore/runtime/JSCell.cpp:158
> +    if (isString())
> +        return static_cast<const JSString*>(this)->toObject(exec, globalObject);
> +    return static_cast<const JSObject*>(this)->toObject(exec, globalObject);

I think it would be better, in the object case, to ASSERT(isObject()) and then static_cast to JSObject*, and remove JSObject::toObject entirely. No need to have that identity function anymore, now that we're not using virtual functions.
Comment 5 Mark Hahnenberg 2011-09-29 14:49:10 PDT
Created attachment 109206 [details]
Patch
Comment 6 Darin Adler 2011-09-29 14:51:11 PDT
Comment on attachment 109206 [details]
Patch

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

> Source/JavaScriptCore/runtime/JSCell.cpp:154
> +JSObject* JSCell::toObject(ExecState* exec, JSGlobalObject* globalObject) const

Should we consider inlining this in JSString.h?
Comment 7 Mark Hahnenberg 2011-09-29 16:34:03 PDT
Created attachment 109217 [details]
Benchmark results (without inlining)
Comment 8 Mark Hahnenberg 2011-09-29 16:34:27 PDT
Created attachment 109218 [details]
Benchmark results (with inlining)
Comment 9 WebKit Review Bot 2011-09-29 19:08:24 PDT
Comment on attachment 109206 [details]
Patch

Clearing flags on attachment: 109206

Committed r96381: <http://trac.webkit.org/changeset/96381>
Comment 10 WebKit Review Bot 2011-09-29 19:08:28 PDT
All reviewed patches have been landed.  Closing bug.