Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 313247

Summary: [performance] Remove the meaningless call to finalize() in CompilationUnitDeclaration
Product: [WebTools] JSDT Reporter: Chris Jaun <cmjaun>
Component: GeneralAssignee: Chris Jaun <cmjaun>
Status: RESOLVED FIXED QA Contact: Nitin Dahyabhai <thatnitind>
Severity: major    
Priority: P3 CC: david_williams
Version: 3.2Keywords: performance
Target Milestone: 3.2 RC2Flags: david_williams: pmc_approved+
thatnitind: pmc_approved? (raghunathan.srinivasan)
thatnitind: pmc_approved? (naci.dai)
thatnitind: pmc_approved? (deboer)
thatnitind: pmc_approved? (neil.hauge)
thatnitind: pmc_approved? (kaloyan)
thatnitind: review+
Hardware: PC   
OS: Windows XP   
Whiteboard: PMC_approved
Attachments:
Description Flags
patch none

Description Chris Jaun CLA 2010-05-17 17:45:04 EDT 
The call to finalize() in CompilationUnitDeclartion performs no function, but is very bad for performance.

We've seen this class holding on to 230+ MB of memory after use.

http://www.ibm.com/developerworks/java/library/j-jtp01274.html - this document in the section "Finalizers are not your friend" provides a good overview of why the finalize() method is bad.

Remove it.
Comment 1 Chris Jaun CLA 2010-05-17 17:46:02 EDT 
Created attachment 168832 [details]
patch
Comment 2 Nitin Dahyabhai CLA 2010-05-17 23:03:07 EDT 
* Explain why you believe this is a stop-ship defect. Or, if it is a "hotbug" (requested by an adopter) please document it as such. 

CompilationUnitDeclarations are created whenever we need to parse/validate each source file.  For larger projects and workspaces this can cause large numbers of these objects, which themselves can be large, to be queued up awaiting their finalize() method to be called by the garbage collector.  It's not difficult for this to cause CPU and memory usage to spike unpleasantly.

* Is there a work-around? If so, why do you believe the work-around is insufficient? 
No, the existence of the method itself is the problem.

* How has the fix been tested? Is there a test case attached to the Bugzilla record? Has a JUnit Test been added? 

Measured using profiling tools.

* Give a brief technical overview. Who has reviewed this fix? 
I've reviewed it.

* What is the risk associated with this fix? 
Zero.  The method being removed is empty.
Comment 3 David Williams CLA 2010-05-18 01:07:24 EDT 
for heavens sakes ... how'd this happen? No need to answer :) just glad it was found and fixed.
Comment 4 Chris Jaun CLA 2010-05-18 09:37:33 EDT 
Patch checked in.