Bug 269978 - [ui] installation history page slow to come up
Summary: [ui] installation history page slow to come up
Status: VERIFIED FIXED
Alias: None
Product: Equinox
Classification: Eclipse Project
Component: p2 (show other bugs)
Version: 3.5   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.6 M3   Edit
Assignee: Susan McCourt CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
Depends on:
Blocks:
 
Reported: 2009-03-25 10:38 EDT by Pascal Rapicault CLA
Modified: 2009-10-27 15:16 EDT (History)
3 users (show)

See Also:


Attachments
Patch (13.86 KB, patch)
2009-05-02 16:19 EDT, Benjamin Cabé CLA
no flags Details | Diff
mylyn/context/zip (3.22 KB, application/octet-stream)
2009-05-02 16:19 EDT, Benjamin Cabé CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Pascal Rapicault CLA 2009-03-25 10:38:28 EDT
M6
My installation history page is rather slow to come up (even though I only have 2 history) and it looks like if it is loading the profile registry in the UI thread.
It is visible because my machine is slow.
Comment 1 Benjamin Cabé CLA 2009-05-02 16:19:02 EDT
Created attachment 134160 [details]
Patch

ProfileSnapshots now implements IDeferredWorkbenchAdapter, so the UI is not blocked anymore while the snapshots are retrieved
In order to keep the behaviour consisting in selecting the first (most recent) snapshot in the viewer, I had to refactor the DeferredQueryContentProvider a bit...
Comment 2 Benjamin Cabé CLA 2009-05-02 16:19:08 EDT
Created attachment 134161 [details]
mylyn/context/zip
Comment 3 Benjamin Cabé CLA 2009-05-04 08:50:12 EDT
CC'ing Susan, she may want to help reviewing the patch :) Since it's RC1, 2 committers need to give their +1: the one that will integrate the patch and another...
Comment 4 Simon Kaegi CLA 2009-05-04 09:41:50 EDT
... reassignment magic as this is in the UI.
Comment 5 Susan McCourt CLA 2009-05-05 11:50:13 EDT
Hi, Benjamin.
Thanks for the patch.  I reviewed it, and it looks like the right solution to me, but wrong timing.  

I think it's too big for an RC1-level change.  While the refactoring is minor, it means we're changing the content provider used in the other views.  If we were simply changing the installation history code and not touching other working code paths, I might consider it.

Unless someone can convince me that this is a critical performance issue for 3.5,  I'm marking this 3.6 to do the right thing (Benjamin's refactoring) rather than hack some kind of surgical fix in for 3.5.
Comment 6 Susan McCourt CLA 2009-09-23 19:41:08 EDT
Comment on attachment 134160 [details]
Patch

I was premature in setting this flag, I didn't use the patch after all.
Comment 7 Susan McCourt CLA 2009-09-24 15:29:10 EDT
Fixed in HEAD >20090924.
Thanks again for the patch, Benjamin.  I updated it to the current code stream and refactored it a little, but ultimately decided I didn't like changing that top view to be a tree and relying on DeferredTreeContentManager (with all its quirks and bugs).  I saw some odd blinkiness and once (only once) saw duplicate elements appear (well described in bug 226343) and then decided to try something simpler.

I added some very basic deferred fetch support for table elements in the ProvElementContentProvider.  Basically it uses IDeferredWorkbenchAdapter to determine if table elements should be retrieved in a job.  With a table, you can do simple removes and adds to do the deferred fetch rather than rely on element collectors and the potential duplicates you get in bug 226343.  I also did a barebones implementation of IDeferredWorkbenchAdapter in ProfileSnapshots, rather than move progress handling into the element building, because the time consuming part of the work is getting the timestamps from the profile registry, which doesn't take a progress monitor anyway.

Added a test case that hammers the content provider to ensure no jobs are leaked or duplicated.
Comment 8 Susan McCourt CLA 2009-10-27 15:14:23 EDT
Verified on WinXP, Build id: I20091027-0100