Summary: | JSC GC lazy sweep does not inline the common cases of cell destruction | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Filip Pizlo <fpizlo> | ||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | fpizlo, oliver, webkit.review.bot | ||||||
Priority: | P2 | ||||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | All | ||||||||
OS: | All | ||||||||
Attachments: |
|
Description
Filip Pizlo
2011-07-18 13:18:35 PDT
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. |