Community
Participate
Working Groups
From bug 389335 I added a println to CommandContributionItem.update(String) System.out.println(getClass().getName() + "(id=" + getId() + ", objectid=" + Integer.toHexString(hashCode()) + ")"); When I first start eclipse with a Java perspective I can open and close Java editors, switch between Java/Git perspective, switch between Java editors all without seeing any calls to CommandContributionItem.update. (Other than initially building the workspace). Now if I open the 'Window -> Show view' menu I see calls to that update. Now every time I do any of the above actions I see multiple calls to update. So if I open a java editor I see the following updates called 12 times! Even something like opening the workspace preferences causes these same updates to be called 18 times! org.eclipse.ui.menus.CommandContributionItem(id=org.eclipse.ant.ui.views.AntView, objectid=1f95fc4) org.eclipse.ui.menus.CommandContributionItem(id=org.eclipse.ui.views.ResourceNavigator, objectid=1e82e1b) org.eclipse.ui.menus.CommandContributionItem(id=org.eclipse.ui.views.ProgressView, objectid=2eab5b) org.eclipse.ui.menus.CommandContributionItem(id=org.eclipse.ui.navigator.ProjectExplorer, objectid=f3a6e) org.eclipse.ui.menus.CommandContributionItem(id=org.eclipse.ui.views.TaskList, objectid=1e154b9) org.eclipse.ui.menus.CommandContributionItem(id=org.eclipse.ui.texteditor.TemplatesView, objectid=602764) Then using the Git perspective really multiplies the problem because so many of their menu items are CommandContributionItems. It just doesn't seem to me that these should need to be updated so many times. Or even updated at all in the above scenarios? Have I found a different bug?
Created attachment 221509 [details] console of CommandContributionItems update switching java editors I am testing with the fix for bug 389335 in my workspace. The attached console output shows my printlns in CommandContributionItem when I have 2 java editors open. Cleared the console and then switched from one java editor to the other by clicking on the editor tab. This was done after I started the workspace, opened the Show View dialog, displayed right-click popup-menus in the various Git views (staging, repositories, reflog, history). You can see in the attachment that the same 20 commands are updated 22 times.
In the commands that have an update called I see in org.eclipse.jface.bindings.BindingManagerEvent.isActiveBindingsChangedFor(ParameterizedCommand) on the last line of that method: !Util.equals(currentBindings, previousBindings) currentBindings is an empty array while previous bindings is null. I think this should return false in this case, while currently it returns true. Then an update of the CommandContributionItem would not occur.
I entered bug 390451 and provided a possible patch solution. I am leaving this bug open because I am still seeing some other seemingly unnecessary calls to update(*) that are unrelated to bug 390451. With the patch for that bug applied I still see a lot of update calls when using the 'Open Type' dialog. This is after having viewed the 'Show View' menu or dialog.
For some more information, I also see multiple update calls when: - 'Open Type' dialog is open and I switch to another application and then back to Eclipse - Opening Preferences (same thing, switching back to eclipse with this open - Open Perspective dialog - Opening a compare editor via a changed file in the Git Staging View - Select a project, Team->Synchronize Workspace (Egit) I don't know if these are all the same problem, but just thought I'd give all the use cases I've uncovered in case they are separate issues.
Created attachment 221532 [details] JVisualVm snapshot - CloseAll 1 java editor I am attaching a couple of updated jvisualvm snapshots. I have the most recent org.eclipse.ui.workbench and org.e4.ui.workbench in my workspace so I have the fix for bug 389335 and the recent fixes related to 389250. I also have my suggested patch for bug 390451 included in my workspace. The first snapshot is using 'close all' to only close 1 java file. The second snapshot is using 'close all' to close 3 java files. I started eclipse and "primed" things by opening a lot of dialogs(show view, show perspective, etc) and right clicking through the egit views. You'll notice that the snapshot of closing 1 file only take .5 seconds while the snapshot closing 3 files takes exponentially longer at 5 seconds! It looks to me like there is a lot of churn around ContextManager.setActiveContextIds
Created attachment 221533 [details] JVisualVm snapshot - CloseAll 3 java editors
Created attachment 221534 [details] console output of ContextManager.setActiveContetIds(Set) I turned on the tracing I noticed in ContextManager.setActiveContetIds(Set). I am attaching the output when I choose to 'close all' on 3 java editors. Notice that it goes back and forth between 2 sets that are almost identical except that first the Set will include these: org.eclipse.ui.textEditorScope, org.eclipse.jdt.ui.javaEditorScope and then next time it is the same Set without those 2 strings. Goes back and forth about 100 times.
Take a look at this call stack. BindingServiceImpl.setContextIds calls ContextManager.setActiveContextIds(). When ContextContextService is asked to getActiveContextIds() it sets the ids on the same ContextManager. Hence the back and forth setting of different active context ids. It short-circuits at the top of this stack since now the activeContextIds sets are equal. I got this stack by opening the 'Open Type' dialog. So in the process of trying to set the activeContextIds to (1) it sets them to (2) instead, which is what they were before. Then you see the same things repeated (3) **** ContextManager.setActiveContextIds(Set) line: 276 **** (3) ContextContextService.getActiveContextIds() line: 83 ContextService.getActiveContextIds() line: 256 ActiveContextSourceProvider.getCurrentState() line: 68 ActiveContextSourceProvider.contextManagerChanged(ContextManagerEvent) line: 50 ContextManager.fireContextManagerChanged(ContextManagerEvent) line: 165 (2) **** ContextManager.setActiveContextIds(Set) line: 293 ***** (2) ContextContextService.getActiveContextIds() line: 83 ContextService.getActiveContextIds() line: 256 ActiveContextSourceProvider.getCurrentState() line: 68 ActiveContextSourceProvider.contextManagerChanged(ContextManagerEvent) line: 50 ContextManager.fireContextManagerChanged(ContextManagerEvent) line: 165 (1) **** ContextManager.setActiveContextIds(Set) line: 293 ***** (1) BindingServiceImpl.setContextIds(Set<String>) line: 237 NativeMethodAccessorImpl.invoke0(Method, Object, Object[]) line: not available [native method] (1) [org.eclipse.jdt.junit.JUnitActionSet, org.eclipse.debug.ui.launchActionSet, org.eclipse.jdt.ui.SearchActionSet, org.eclipse.ui.contexts.dialogAndWindow, org.eclipse.jdt.ui.A_OpenActionSet, org.eclipse.ui.edit.text.actionSet.navigation, org.eclipse.ui.NavigateActionSet, org.eclipse.ui.actionSet.openFiles, org.eclipse.jdt.debug.ui.JDTDebugActionSet, org.eclipse.ui.actionSet.keyBindings, org.eclipse.ui.edit.text.actionSet.annotationNavigation, org.eclipse.ui.edit.text.actionSet.convertLineDelimitersTo, org.eclipse.search.searchActionSet, org.eclipse.ui.cheatsheets.actionSet, org.eclipse.ui.externaltools.ExternalToolsSet, org.eclipse.ui.contexts.window, org.eclipse.jdt.ui.JavaElementCreationActionSet, org.eclipse.debug.ui.breakpointActionSet, org.eclipse.jdt.ui.JavaActionSet, org.eclipse.jdt.ui.CodingActionSet] (2) [org.eclipse.ui.contexts.dialog, org.eclipse.ui.contexts.dialogAndWindow] (3) [org.eclipse.ui.contexts.dialog, org.eclipse.ui.contexts.dialogAndWindow] suspicious code is in ContextContextService: public Collection<String> getActiveContextIds() { Set<String> set = (Set<String>) eclipseContext .get(IServiceConstants.ACTIVE_CONTEXTS); **if (set != null) { ** contextManager.setActiveContextIds(set); **} return set; }
(In reply to comment #2) > In the commands that have an update called I see in > org.eclipse.jface.bindings.BindingManagerEvent. > isActiveBindingsChangedFor(ParameterizedCommand) on the last line of that > method: I've pushed a fix for this case in http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?h=R4_2_maintenance&id=e1c3aa70ff824eb7101660c4d7f977f2d673c1a8 (In reply to comment #8) > Take a look at this call stack. BindingServiceImpl.setContextIds calls > ContextManager.setActiveContextIds(). When ContextContextService is asked to > getActiveContextIds() it sets the ids on the same ContextManager. Hence the > back and forth setting of different active context ids. It short-circuits at > the top of this stack since now the activeContextIds sets are equal. I got > this stack by opening the 'Open Type' dialog. So in the process of trying to > set the activeContextIds to (1) it sets them to (2) instead, which is what > they were before. Then you see the same things repeated Bogdan, you were also looking at the excessive calls to contextManager.setActiveContextIds(*). This call should only be made once, by the service that belongs to the activePart. Right now it seems it's being set by every part that's interacted with the IContextService. PW
Bogdan has a fix for this in bug 391868 PW
(In reply to comment #10) > Bogdan has a fix for this in bug 391868 > > PW I can't see any fix in said bug ;-)
Released a stability improvement to the EContextService. http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?h=R4_2_maintenance&id=37b36637d884fce50f35ee528789b5f1c1b9dd3d - Improve the add/remove time buy using a LinkedList. - Only update the object in the context if it would add a new ID to the activeContexts (don't fire a change unless necessary) - ref count the IDs by keeping them in the LinkedList PW
Here is a link to a topic branch that has a commit that defers some context updates on part activation. I've tested switching between xml editors on both Windows and Linux and get a 20% improvement in part activation with this patch. http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?h=gheorghe/contextDeferUpdates&id=e7d7c1533308013507bc092d9bda787ea974f99d
(In reply to comment #13) > Here is a link to a topic branch that has a commit that defers some context > updates on part activation. I've tested switching between xml editors on > both Windows and Linux and get a 20% improvement in part activation with > this patch. Wow, thanks Bogdan. I've done some testing, and it looks good. I've released it to R4_2_maintenance (added a minor change) for some more widespread testing and used it to fix one of the other bugs, bug 384758 Thanx, PW
Marking fixed based on the commit released in comment #14.
In M20130116-1800 PW
*** Bug 384914 has been marked as a duplicate of this bug. ***
