Bug 68937

Summary: De-virtualize JSCell::toObject
Product: WebKit Reporter: Mark Hahnenberg <mhahnenberg>
Component: JavaScriptCoreAssignee: Mark Hahnenberg <mhahnenberg>
Status: RESOLVED FIXED    
Severity: Normal CC: ggaren, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 67690    
Attachments:
Description Flags
Patch
none
Patch
none
Patch
none
Patch
none
Benchmark results (without inlining)
none
Benchmark results (with inlining) none

Mark Hahnenberg
Reported 2011-09-27 14:32:37 PDT
Part of process of de-virtualizing JSCell.
Attachments
Patch (5.85 KB, patch)
2011-09-27 14:40 PDT, Mark Hahnenberg 		no flags
DetailsFormatted DiffDiff
Patch (6.94 KB, patch)
2011-09-28 11:32 PDT, Mark Hahnenberg 		no flags
DetailsFormatted DiffDiff
Patch (6.94 KB, patch)
2011-09-28 11:58 PDT, Mark Hahnenberg 		no flags
DetailsFormatted DiffDiff
Patch (8.72 KB, patch)
2011-09-29 14:49 PDT, Mark Hahnenberg 		no flags
DetailsFormatted DiffDiff
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
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Mark Hahnenberg
Comment 1 2011-09-27 14:40:49 PDT
Created attachment 108902 [details] Patch
Mark Hahnenberg
Comment 2 2011-09-28 11:32:54 PDT
Created attachment 109041 [details] Patch
Mark Hahnenberg
Comment 3 2011-09-28 11:58:01 PDT
Created attachment 109044 [details] Patch
Geoffrey Garen
Comment 4 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.
Mark Hahnenberg
Comment 5 2011-09-29 14:49:10 PDT
Created attachment 109206 [details] Patch
Darin Adler
Comment 6 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?
Mark Hahnenberg
Comment 7 2011-09-29 16:34:03 PDT
Created attachment 109217 [details] Benchmark results (without inlining)
Mark Hahnenberg
Comment 8 2011-09-29 16:34:27 PDT
Created attachment 109218 [details] Benchmark results (with inlining)
WebKit Review Bot
Comment 9 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>
WebKit Review Bot
Comment 10 2011-09-29 19:08:28 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.