WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
64745
JSC GC lazy sweep does not inline the common cases of cell destruction
https://bugs.webkit.org/show_bug.cgi?id=64745
Summary
JSC GC lazy sweep does not inline the common cases of cell destruction
Filip Pizlo
Reported
2011-07-18 13:18:35 PDT
Most objects in the heap are instances of JSFinalObject, which has a relatively straight-forward destructor. This destructor's common case is to do nothing: the property storage is not inline, so there is nothing to destroy. However, the lazy sweep will currently always make a slow virtual call because it doesn't special-case the common cases.
Attachments
the patch
(2.12 KB, patch)
2011-07-18 13:27 PDT
,
Filip Pizlo
oliver
: review-
oliver
: commit-queue-
Details
Formatted Diff
Diff
the patch (fix review)
(1.79 KB, patch)
2011-07-18 14:33 PDT
,
Filip Pizlo
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Filip Pizlo
Comment 1
2011-07-18 13:27:16 PDT
Created
attachment 101197
[details]
the patch
Oliver Hunt
Comment 2
2011-07-18 13:53:48 PDT
Comment on
attachment 101197
[details]
the patch View in context:
https://bugs.webkit.org/attachment.cgi?id=101197&action=review
r- :(
> Source/JavaScriptCore/heap/MarkedBlock.cpp:96 > + if (*reinterpret_cast<void**>(cell) == jsFinalObjectVPtr) {
use cell->vptr();
> Source/JavaScriptCore/heap/MarkedBlock.cpp:99 > + if (!object->isUsingInlineStorage()) > + delete [] object->m_propertyStorage;
object->JSFinalObject::~JSFinalObject() didn't work?
Filip Pizlo
Comment 3
2011-07-18 13:59:07 PDT
(In reply to
comment #2
)
> (From update of
attachment 101197
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=101197&action=review
> > r- :( > > > Source/JavaScriptCore/heap/MarkedBlock.cpp:96 > > + if (*reinterpret_cast<void**>(cell) == jsFinalObjectVPtr) { > > use cell->vptr();
Ah, that's handy!
> > Source/JavaScriptCore/heap/MarkedBlock.cpp:99 > > + if (!object->isUsingInlineStorage()) > > + delete [] object->m_propertyStorage; > > object->JSFinalObject::~JSFinalObject() didn't work?
I tried object->~JSFinalObject() but that didn't give a speed-up. I forgot the object->JSFinalObject::~JSFinalObject syntax. That works great. Doing some quick tests and will resubmit shortly...
Filip Pizlo
Comment 4
2011-07-18 14:33:43 PDT
Created
attachment 101205
[details]
the patch (fix review)
Darin Adler
Comment 5
2011-07-18 14:41:07 PDT
Comment on
attachment 101205
[details]
the patch (fix review) View in context:
https://bugs.webkit.org/attachment.cgi?id=101205&action=review
> Source/JavaScriptCore/heap/MarkedBlock.cpp:98 > + JSFinalObject* object = reinterpret_cast<JSFinalObject*>(cell); > + object->JSFinalObject::~JSFinalObject();
I would have done this as a one liner: reinterpret_cast<JSFinalObject*>(cell)->object->JSFinalObject::~JSFinalObject() I also think it deserves a “why” comment, stating this is a special case for speed.
WebKit Review Bot
Comment 6
2011-07-18 15:56:00 PDT
Comment on
attachment 101205
[details]
the patch (fix review) Clearing flags on attachment: 101205 Committed
r91218
: <
http://trac.webkit.org/changeset/91218
>
WebKit Review Bot
Comment 7
2011-07-18 15:56:04 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.
Top of Page
Format For Printing
XML
Clone This Bug