Bug 21321 – speed up JavaScriptCore by inlining Heap in JSGlobalData

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Darin Adler]

There's no reason that JSGlobalData should have a Heap* -- instead it should just contain the heap.

[Darin Adler]

Created attachment 24032 [details]
patch

[Geoffrey Garen]

Comment on attachment 24032 [details]
patch

Gotta destroy the heap before any other JSGlobalData tear-down happens.

[Darin Adler]

Created attachment 24033 [details]
patch, with better destruction

[Geoffrey Garen]

Comment on attachment 24033 [details]
patch, with better destruction

-    RefPtr<JSGlobalData> protect(m_globalData);

Put this back and r=me.

[Darin Adler]

http://trac.webkit.org/changeset/37215

[Alexey Proskuryakov]

The JSGlobalData destructor calls Heap::destroy(), which in turn ref's JSGlobalData. Normally, this should  cause an assertion in RefPtr::ref(), due to referencing an object that is being destroyed. This doesn't happen because (1) WebCore never destroys its global data and (2) API clients first destroy the heap in JSGlobalContextRelease(), so Heap::destroy() becomes a no-op.

This is safe in practice and a pre-existing issue (the code already did call Heap destructor from JSGlobalData destructor).

I think that the contract should be that clients should destroy the heap before releasing the last JSGlobalData reference.