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

Bug 348798

Summary: The timer code in HandledContributionItem creates enourmous numbers of GC'able objects
Product: [Eclipse Project] e4 Reporter: Eric Moffatt <emoffatt>
Component: UIAssignee: Paul Webster <pwebster>
Status: RESOLVED FIXED QA Contact:
Severity: major    
Priority: P3 CC: bokowski, john.arthorne, Mike_Wilson, pwebster, remy.suen
Version: unspecifiedFlags: Mike_Wilson: pmc_approved+
remy.suen: review+
Target Milestone: 4.1 RC4   
Hardware: PC   
OS: Windows XP   
Whiteboard:
Attachments:
Description Flags
Cache the static context per HCI v01 none

Description Eric Moffatt CLA 2011-06-08 16:03:29 EDT 
If you open the HeapStatus you can watch it grow by over a meg a second. While it will eventually GC this memory the result is that the garbage collector is 'locking' the system for longer than most apps due to the amount of work it has to do.

We know that it's the timerExec because commenting out the code to restart the timer (lines 99-100) stops the heap from growing...

Paul, I'm not sure whether this is actually fixable but we ought to look into it anyway to see if we can re-use the contexts.
Comment 1 Eric Moffatt CLA 2011-06-08 16:05:10 EDT 
I've just tried to see what the gc's performance impact is and it doesn't appear to be that major.
Comment 2 Paul Webster CLA 2011-06-08 16:31:12 EDT 
My guess is that its because we're creating a static context per item, per 400 ms call.

We dispose it right away so that it's available for GC ASAP, but as you noticed it continues to climb for a while.

The other option would be to allocate the context per HCI instance, and then we only have to fill in the SWT Event object (or remove it as appropriate).  That will stop the constant creation/destruction, but those contexts will show up in the Contexts debug view as they'll live until the HCI.dispose().

PW
Comment 3 Paul Webster CLA 2011-06-09 12:15:52 EDT 
Created attachment 197701 [details]
Cache the static context per HCI v01

This creates one staticContext that is re-used and updated with correct information on every timer check for toolitems.

It slows the heap growth considerably.  30 Meg in the time it took my outer to grow 100 meg.  The heap still grows as long as the timerExec(*) is running, but we don't create any more temp objects in this code.  Even the safe runner is re-used per instance.

PW
Comment 4 Paul Webster CLA 2011-06-09 12:17:33 EDT 
Remy, could you review please?

PW
Comment 5 Remy Suen CLA 2011-06-09 13:34:35 EDT 
(In reply to comment #3)
> Created attachment 197701 [details]
> Cache the static context per HCI v01

This looks good to me. I don't see any behaviour indicative of rabid heap growth.

Please delete the commented out line 318.

One thing I noticed is that the context's interfaces are "static" but I am guessing that when a different model is inserted via setModel(MHandledItem) that a new contribution item would be spawned (with a new context being generated).
Comment 6 Paul Webster CLA 2011-06-09 16:09:07 EDT 
We're considering this for RC4
PW
Comment 7 Mike Wilson CLA 2011-06-09 16:42:14 EDT 
Fix creates the context once and deletes it at the end.
Comment 8 Paul Webster CLA 2011-06-09 16:59:46 EDT 
Released for post RC4.
PW