Summary: | De-virtualize JSCell::toObject | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mark Hahnenberg <mhahnenberg> | ||||||||||||||
Component: | JavaScriptCore | Assignee: | 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
Mark Hahnenberg
2011-09-27 14:32:37 PDT
Created attachment 108902 [details]
Patch
Created attachment 109041 [details]
Patch
Created attachment 109044 [details]
Patch
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. Created attachment 109206 [details]
Patch
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? Created attachment 109217 [details]
Benchmark results (without inlining)
Created attachment 109218 [details]
Benchmark results (with inlining)
Comment on attachment 109206 [details] Patch Clearing flags on attachment: 109206 Committed r96381: <http://trac.webkit.org/changeset/96381> All reviewed patches have been landed. Closing bug. |