Community
Participate
Working Groups
+++ This bug was initially created as a clone of Bug #249995 +++ Looking at a heapdump of a long running RCP based application I see org.eclipse.ui.internal.contexts.SlaveContextService retaining 4MB or 8.7% of the Java heap. Digging deeper I see there are 62,389 instances of org.eclipse.ui.internal.contexts.ContextActivation. I think I understand why there are so many instances, I was able to "recreate" with the "RCP Mail template" example. I changed the OpenViewAction.run method to do the following: for (int i = 0; i < 100; i++) window.getActivePage().hideActionSet(getActionDefinitionId()); before running the for loop I took a look at SlaveContextService.fLocalActivations, it was size 3, which is expected. After running this for loop twice I see 400 new instances of ContextActivation in the map. service org.eclipse.ui.internal.contexts.SlaveContextService (id=58) 'service' referenced from: fContextManagerListeners java.util.ArrayList<E> (id=80) fDefaultExpression org.eclipse.ui.internal.expressions.WorkbenchWindowExpression (id=82) fLocalActivations java.util.HashMap<K,V> (id=84) [0...99] [100...199] [200...299] [300...399] [400...403] As I understand the hideActionSet code this is expected since hideActionSet calls Perspective.removeActionSet which contains: IContextService service = (IContextService)page.getWorkbenchWindow().getService(IContextService.class); .... service.activateContext(ContextAuthority.DEFER_EVENTS); service.activateContext(ContextAuthority.SEND_EVENTS); Since the workbench window is global, and deactivateContext isn't called, these will continue to accumulate until the workbench is exited. In the removeActionSet case it would be straightforward to deactivate in the finally block but wondered if I was misunderstanding the code.
This can probably be addressed as a method, deferUpdates(boolean). Let's start with the method on ContextService/SlaveContextService, and then we'll see if we should push it to IContextService. PW
Created attachment 115916 [details] Patch v01 Paul, I've attached the patch. Please review and let me know is this the right way to proceed.
(In reply to comment #2) > Created an attachment (id=115916) [details] > Patch v01 I think this is the update action patch. PW
Created attachment 116013 [details] Patch v01 - real one :-) Sorry for that. Attaching the right file
OK, I think what we want to do is replace all our calls to: service.activateContext(ContextAuthority.DEFER_EVENTS); service.activateContext(ContextAuthority.SEND_EVENTS); with calls to service.deferUpdates(true/false). Basically, let's remove all references to DEFER_EVENTS/SEND_EVENTS completely. This should be pushed all the way through the system to hook up with setEventCaching(*), which can probably be renamed to deferUpdates(*) PW
Created attachment 116275 [details] Patch v02 Somehow I felt sendUpdates() is better than deferUpdates(false). I've added the methods in the API. Please review and comment.
(In reply to comment #6) > Created an attachment (id=116275) [details] > Patch v02 > > Somehow I felt sendUpdates() is better than deferUpdates(false). I've added the > methods in the API. Please review and comment. > OK, some comments: 1) please delete the code you remove instead of commenting it out (CVS will show us the diff if we need it). 2) simply use deferUpdates(true/false) to be consistent with other uses within the workbench. 3) The I don't know where those SEND/DEFER activations are comming from (since we never applied that patch in HEAD). SlaveContextService should simply call parent.deferUpdates(defer) and there should be no more references to the SEND/DEFER activations or IDs PW
Created attachment 116621 [details] Patch v03 I've re-spun the patch to make it API Tooling correct (more or less) and remove errors that I get when applying. I'll send a note about the API tooling. PW
Created attachment 116928 [details] Patch v04 Patch updated: (1) Dead code is removed rather than commenting out (2) sendUpdates is now deferUpdate(false) (3) SlaveContextService calls the parent service's deferUpdates
I added a minor update of the javadoc and released to HEAD >20081107 PW
In I20081209-0100 PW
Looks fine to me; the code is far cleaner with this implementation than it was...
