Bug 21321

Summary: speed up JavaScriptCore by inlining Heap in JSGlobalData
Product: WebKit Reporter: Darin Adler <darin>
Component: JavaScriptCoreAssignee: Darin Adler <darin>
Status: RESOLVED FIXED    
Severity: Normal CC: ap
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: OS X 10.5   
Attachments:
Description Flags
patch
ggaren: review-
patch, with better destruction ggaren: review+

Darin Adler
Reported 2008-10-02 16:16:47 PDT
There's no reason that JSGlobalData should have a Heap* -- instead it should just contain the heap.
Attachments
patch (25.70 KB, patch)
2008-10-02 16:22 PDT, Darin Adler 		ggaren: review-
DetailsFormatted DiffDiff
patch, with better destruction (27.66 KB, patch)
2008-10-02 16:39 PDT, Darin Adler 		ggaren: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Darin Adler
Comment 1 2008-10-02 16:22:12 PDT
Created attachment 24032 [details] patch
Geoffrey Garen
Comment 2 2008-10-02 16:32:38 PDT
Comment on attachment 24032 [details] patch Gotta destroy the heap before any other JSGlobalData tear-down happens.
Darin Adler
Comment 3 2008-10-02 16:39:33 PDT
Created attachment 24033 [details] patch, with better destruction
Geoffrey Garen
Comment 4 2008-10-02 16:43:10 PDT
Comment on attachment 24033 [details] patch, with better destruction - RefPtr<JSGlobalData> protect(m_globalData); Put this back and r=me.
Darin Adler
Comment 5 2008-10-02 16:49:53 PDT
http://trac.webkit.org/changeset/37215
Alexey Proskuryakov
Comment 6 2008-10-02 23:01:49 PDT
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.
Note You need to log in before you can comment on or make changes to this bug.