Bug 144294 - Performance: CommonNavigator could easily cache its ISelection to improve its performance
Summary: Performance: CommonNavigator could easily cache its ISelection to improve its...
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.2   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.2.1   Edit
Assignee: Boris Bokowski CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
Depends on:
Blocks:
 
Reported: 2006-05-29 15:14 EDT by Yasser Lulu CLA
Modified: 2006-08-31 14:32 EDT (History)
5 users (show)

See Also:

Attachments
caching ISelection in CommonNavigator (4.50 KB, patch)
2006-05-29 15:15 EDT, Yasser Lulu CLA 		no flags Details | Diff
proposed patch (3.53 KB, patch)
2006-07-31 06:11 EDT, Boris Bokowski CLA 		no flags Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Yasser Lulu CLA 2006-05-29 15:14:20 EDT 
Currently, the CommonNavigator does not override the getSelection() method, which is quite expensive when called repeatedly by clients since it gets the current selection from the underlying SWT widget each time it gets called. A simple trick is to cache the selection and update it only when it actually changes. This would result in improved performance for clients who call getSelection() often. This defect is related to 140032. Please refer to it for more info. I will be submitting a patch shortly.
Comment 1 Yasser Lulu CLA 2006-05-29 15:15:15 EDT 
Created attachment 42885 [details]
caching ISelection in CommonNavigator
Comment 2 Michael Valenta CLA 2006-05-31 11:54:16 EDT 
-1. McQ, Boris and I have discussed this. This is a problem in TreeViewer so any fix should be made there. Please respond on bug 140032.
Comment 3 Boris Bokowski CLA 2006-07-13 07:49:35 EDT 
Michael, in the meantime we have looked at the issue together with Yasser and for 3.2.1, the safest fix on the Platform side seems to be caching the selection in the common navigator.  We should talk about this.
Comment 4 Boris Bokowski CLA 2006-07-13 07:49:56 EDT 
Forgot to reopen...
Comment 5 Michael D. Elder CLA 2006-07-19 16:28:36 EDT 
Tentatively scheduling this for 3.2.1 so it doesn't fall off of the radar. 

Boris -- drop me a line when you're back.
Comment 6 Boris Bokowski CLA 2006-07-31 06:11:51 EDT 
Created attachment 47059 [details]
proposed patch

Michael, could you please review this patch for inclusion in 3.2.1?  It is not as simple as I had hoped - we should get others to review the patch (or an improved patch) as well.
Comment 7 Yasser Lulu CLA 2006-08-14 15:19:25 EDT 
I tried the patch on my machine and it seems fine in terms of performance
Comment 8 Michael D. Elder CLA 2006-08-15 13:44:12 EDT 
I see no issues with this patch. OK to release.
Comment 9 Tod Creasey CLA 2006-08-16 12:47:55 EDT 
Has this been released yet? The 3.2.1 release candidates are starting next week.
Comment 10 Boris Bokowski CLA 2006-08-16 15:57:35 EDT 
I missed this one for M20060816-0800 because I forgot to assign it to me.
Comment 11 Boris Bokowski CLA 2006-08-16 16:28:48 EDT 
Released to HEAD and R3_2_maintenance.
Comment 12 Boris Bokowski CLA 2006-08-31 14:32:07 EDT 
Verified that the fix is in the 3.2.1 stream [M20060830-0080] and in HEAD.