Bug 313247 - [performance] Remove the meaningless call to finalize() in CompilationUnitDeclaration
Summary: [performance] Remove the meaningless call to finalize() in CompilationUnitDec...
Status: RESOLVED FIXED
Alias: None
Product: JSDT
Classification: WebTools
Component: General (show other bugs)
Version: 3.2   Edit
Hardware: PC Windows XP
: P3 major (vote)
Target Milestone: 3.2 RC2   Edit
Assignee: Chris Jaun CLA
QA Contact: Nitin Dahyabhai CLA
URL:
Whiteboard: PMC_approved
Keywords: performance
Depends on:
Blocks:
 
Reported: 2010-05-17 17:45 EDT by Chris Jaun CLA
Modified: 2010-05-19 23:56 EDT (History)
1 user (show)

See Also:
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+


Attachments
patch (1.49 KB, patch)
2010-05-17 17:46 EDT, Chris Jaun CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
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.