Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 370337 - An IContributionManagerOverrides's getVisible(*) method is called 2k-10k times
Summary: An IContributionManagerOverrides's getVisible(*) method is called 2k-10k times
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.2   Edit
Hardware: PC Windows 7
: P3 normal (vote)
Target Milestone: 4.2.2   Edit
Assignee: Platform-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
: 370758 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-02-01 10:20 EST by Remy Suen CLA
Modified: 2013-01-23 13:27 EST (History)
8 users (show)

See Also:


Attachments
Menu contributions patch v1 (2.99 KB, patch)
2012-02-01 13:52 EST, Remy Suen CLA
no flags Details | Diff
Context patch v1 (8.21 KB, patch)
2012-02-01 15:03 EST, Remy Suen CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Remy Suen CLA 2012-02-01 10:20:01 EST
While looking at the performance problem behind bug 320478, I have noticed that the getVisible(*) method is called 2k-10k times in 4.x when switching between active parts. This number is less than 100 in 3.x and only goes into the hundreds when you switch perspectives.
Comment 1 Remy Suen CLA 2012-02-01 10:47:14 EST
The number varies greatly but some items get called over 160 times.
Comment 2 Remy Suen CLA 2012-02-01 11:01:07 EST
WorkbenchPage's updateActionSets(*) method seems to be the problem. It gets at least five or six times slower when we apply the patch from bug 320478. The problem probably lies in the fact that we have to activate and deactivate contexts individually. This means that context changes are made incrementally instead of in a batch, compounding the number of change operations required for showing and hiding action sets.
Comment 3 Remy Suen CLA 2012-02-01 12:51:13 EST
(In reply to comment #2)
> The
> problem probably lies in the fact that we have to activate and deactivate
> contexts individually. This means that context changes are made incrementally
> instead of in a batch, compounding the number of change operations required for
> showing and hiding action sets.

I tried batching the changes and while it's cut the time by more than half, the method is now called 3k-5k times (higher minimum, much lower maximum).
Comment 4 Remy Suen CLA 2012-02-01 13:16:57 EST
(In reply to comment #3)
> I tried batching the changes and while it's cut the time by more than half, the
> method is now called 3k-5k times (higher minimum, much lower maximum).

Further changes in MenuManagerRenderer and ContributionRecord brings it down to the 1k range.
Comment 5 Remy Suen CLA 2012-02-01 13:52:56 EST
Created attachment 210396 [details]
Menu contributions patch v1

This patch changes the menu managers to not get updated so aggressively.
Comment 6 Remy Suen CLA 2012-02-01 15:03:41 EST
Created attachment 210400 [details]
Context patch v1

This patch batches the context change operations. Though it's not clear to me if it's actually in line with what deferUpdates(*) is supposed to do or not.
Comment 7 Remy Suen CLA 2012-02-01 19:09:24 EST
(In reply to comment #5)
> Created attachment 210396 [details]
> Menu contributions patch v1

The same change should also be applied to ToolBarManagerRenderer.
Comment 8 Remy Suen CLA 2012-02-02 08:29:10 EST
(In reply to comment #7)
> The same change should also be applied to ToolBarManagerRenderer.

Paul has given me the OK. Both renderers and contribution records have been updated.
http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=990da1fcd2b8588a64bae4cf7efb1f2f156583f3
Comment 9 Oleg Besedin CLA 2012-02-06 16:53:43 EST
*** Bug 370758 has been marked as a duplicate of this bug. ***
Comment 10 Paul Webster CLA 2013-01-18 11:22:29 EST
(In reply to comment #2)
> WorkbenchPage's updateActionSets(*) method seems to be the problem. It gets
> at least five or six times slower when we apply the patch from bug 320478.
> The problem probably lies in the fact that we have to activate and
> deactivate contexts individually. This means that context changes are made
> incrementally instead of in a batch, compounding the number of change
> operations required for showing and hiding action sets.

The context changes are now all batched, we're down to about ~150 for active part changes.

PW
Comment 11 Dani Megert CLA 2013-01-23 05:58:48 EST
(In reply to comment #10)
> The context changes are now all batched, we're down to about ~150 for active
> part changes.
> 
> PW

Verified in N20130122-2000-