Bug 209097 - [Viewers] TreeViewer: Refreshing from a large number of entries to a small number of entries very slow
Summary: [Viewers] TreeViewer: Refreshing from a large number of entries to a small nu...
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows Vista
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Platform UI Triaged CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
Depends on:
Blocks: 208989
  Show dependency tree
 
Reported: 2007-11-07 16:12 EST by Tod Creasey CLA
Modified: 2020-09-25 04:08 EDT (History)
6 users (show)

See Also:


Attachments
Patch (2.35 KB, patch)
2007-11-09 15:39 EST, Tod Creasey CLA
no flags Details | Diff
Updated patch (2.35 KB, patch)
2007-11-16 08:15 EST, Tod Creasey CLA
no flags Details | Diff
Patch with the newest code (20.26 KB, text/file)
2007-11-16 08:17 EST, Tod Creasey CLA
no flags Details
patch without formatting (2.58 KB, patch)
2007-11-16 08:20 EST, Tod Creasey CLA
no flags Details | Diff
patch (2.60 KB, text/file)
2007-11-16 10:46 EST, Tod Creasey CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Tod Creasey CLA 2007-11-07 16:12:10 EST
M3

When you refresh a large tree viewer (> 100 items) with a small amount of content (1 or 2) the deletions of the tree elements is very slow.

Boris optimized the Quick Access tree to remove all entries when the change in content size was going to be high and got significant performance improvements = I am seeing similar things in the Markers view.

We should see if this optimization can be generally applied to tree viewers.
Comment 1 Tod Creasey CLA 2007-11-09 15:39:34 EST
Created attachment 82568 [details]
Patch

This time changes the time to execute the switch in Bug 208989 from 11s to 370 ms. In this patch I delete all of the items if there the new size is 20% of the old size.
Comment 2 Markus Keller CLA 2007-11-15 06:09:09 EST
(In reply to comment #1)
> Created an attachment (id=82568) [details]

The change in TreeViewer has probably caused bug 209919.
Comment 3 Tod Creasey CLA 2007-11-16 08:15:20 EST
Created attachment 83062 [details]
Updated patch

This patch uses the tree viewer removeAll API and only does the optimized refresh if it is the root to prevent flashing when updating an intermediate node.

My performance test (ShrinkingTreeTest) indicates that this is roughly as performant as the more aggressive solution in the first patch.

If you look at Markus's example from Bug 209919 without the check for the element being updated being the root you will get a lot of flash.

Here are his steps again

Steps:
- org.eclipse.core.expressions from CVS
- create new package p
- drag all *.java files from package org.eclipse.core.expressions to p
- click OK
Comment 4 Tod Creasey CLA 2007-11-16 08:17:03 EST
Created attachment 83063 [details]
Patch with the newest code

previous patch was out of date as I thought it had been updated but I need to hit OK in Eclipse first - use this one.
Comment 5 Tod Creasey CLA 2007-11-16 08:20:19 EST
Created attachment 83064 [details]
patch without formatting

Same patch without formatting changes so it is easier to browse
Comment 6 Tod Creasey CLA 2007-11-16 10:46:40 EST
Created attachment 83075 [details]
patch

The previous patch broke some suites as I was relying on removeAll in the TreeViewer to clean up the caching (which it doesn't). 

Here is a patch that does the cleanup.
Comment 7 Tod Creasey CLA 2007-11-16 14:38:01 EST
Be sure to test switching the MarkersView to tasks and then switching from sorting by type and back to none as this is how I found many problems when working on this.
Comment 8 Boris Bokowski CLA 2008-05-02 14:56:51 EDT
Mass update - removing 3.4 target. This was one of the bugs I marked for investigation (and potential fixing) in 3.4 but I ran out of time. Please ping on the bug if fixing it would be really important for 3.4, and does not require API changes or feature work.
Comment 9 Boris Bokowski CLA 2009-11-26 09:54:29 EST
Hitesh is now responsible for watching bugs in the [Viewers] component area.
Comment 10 Eclipse Webmaster CLA 2019-09-06 15:37:55 EDT Comment hidden (obsolete)
Comment 11 Lars Vogel CLA 2019-11-06 02:16:25 EST
Karsten, Michael or Paul, can you check if patch from https://bugs.eclipse.org/bugs/show_bug.cgi?id=209097#c6 is still relevant?
Comment 12 Lars Vogel CLA 2020-09-25 04:06:39 EDT
Using org.eclipse.jface.tests.performance.ShrinkingTreeTest is looks like this patch is still valid:

Without the patch:


Scenario 'org.eclipse.jface.tests.performance.ShrinkingTreeTest#testTreeViewerRefresh()' (average over 100 samples):
  System Time:              19ms        (95% in [18ms, 21ms])          Measurable effect: 2ms (0.4 SDs) (required sample size for an effect of 5% of mean: 657)
  Used Java Heap:      -507.77K         (95% in [-2.64M, 1.65M])       Measurable effect: 4.32M (0.4 SDs) (required sample size for an effect of 5% of stdev: 6400)
  Working Set:           19.72K         (95% in [-3.33K, 42.77K])      Measurable effect: 46.47K (0.4 SDs) (required sample size for an effect of 5% of stdev: 6400)
  Elapsed Process:          19ms        (95% in [18ms, 21ms])          Measurable effect: 2ms (0.4 SDs) (required sample size for an effect of 5% of mean: 657)
  Kernel time:               0ms        (95% in [0ms, 0ms])            Measurable effect: 0ms (0.4 SDs) (required sample size for an effect of 5% of stdev: 6401)
  CPU Time:                 26ms        (95% in [20ms, 31ms])          Measurable effect: 10ms (0.4 SDs) (required sample size for an effect of 5% of stdev: 6401)
  Hard Page Faults:          0          (95% in [0, 0])               
  Soft Page Faults:          9          (95% in [0, 19])               Measurable effect: 19 (0.4 SDs) (required sample size for an effect of 5% of stdev: 6401)
  Text Size:                 0          (95% in [0, 0])               
  Data Size:                 0          (95% in [0, 0])               
  Library Size:           3.44K         (95% in [440, 6.45K])          Measurable effect: 6.07K (0.4 SDs) (required sample size for an effect of 5% of stdev: 6400)


With the patch

Scenario 'org.eclipse.jface.tests.performance.ShrinkingTreeTest#testTreeViewerRefresh()' (average over 100 samples):
  System Time:               6ms        (95% in [5ms, 6ms])            Measurable effect: 0ms (0.4 SDs) (required sample size for an effect of 5% of mean: 394)
  Used Java Heap:       578.62K         (95% in [544.97K, 612.27K])    Measurable effect: 67.84K (0.4 SDs) (required sample size for an effect of 5% of mean: 550)
  Working Set:           35.16K         (95% in [3.04K, 67.27K])       Measurable effect: 64.74K (0.4 SDs) (required sample size for an effect of 5% of stdev: 6401)
  Elapsed Process:           6ms        (95% in [5ms, 6ms])            Measurable effect: 0ms (0.4 SDs) (required sample size for an effect of 5% of mean: 394)
  Kernel time:               0ms        (95% in [0ms, 0ms])            Measurable effect: 0ms (0.4 SDs) (required sample size for an effect of 5% of stdev: 6400)
  CPU Time:                  8ms        (95% in [6ms, 10ms])           Measurable effect: 3ms (0.4 SDs) (required sample size for an effect of 5% of stdev: 6401)
  Hard Page Faults:          0          (95% in [0, 0])               
  Soft Page Faults:         18          (95% in [5, 31])               Measurable effect: 26 (0.4 SDs) (required sample size for an effect of 5% of stdev: 6401)
  Text Size:                 0          (95% in [0, 0])               
  Data Size:                 0          (95% in [0, 0])               
  Library Size:          23.16K         (95% in [-9.81K, 56.13K])      Measurable effect: 66.47K (0.4 SDs) (required sample size for an effect of 5% of stdev: 6401)

I convert the patch into a Gerrit so that people familier with the viewer coding can have a look.
Comment 13 Eclipse Genie CLA 2020-09-25 04:08:08 EDT
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/169884