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.
Created attachment 101197 [details] the patch
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?
(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...
Created attachment 101205 [details] the patch (fix review)
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.
Comment on attachment 101205 [details] the patch (fix review) Clearing flags on attachment: 101205 Committed r91218: <http://trac.webkit.org/changeset/91218>
All reviewed patches have been landed. Closing bug.