Bug 99002 - [Presentations] Resize tests are slower since M7
Summary: [Presentations] Resize tests are slower since M7
Status: RESOLVED WORKSFORME
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.1   Edit
Hardware: PC Windows XP
: P5 normal (vote)
Target Milestone: ---   Edit
Assignee: Platform UI Triaged CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
Depends on:
Blocks:
 
Reported: 2005-06-08 12:41 EDT by Tod Creasey CLA
Modified: 2020-08-04 05:07 EDT (History)
6 users (show)

See Also:


Attachments
Profiler output for the resource perspective resize test, code in head (8.10 KB, application/octet-stream)
2005-06-09 16:09 EDT, Stefan Xenos CLA
no flags Details
Profiler output for the resize tests in m7 (7.84 KB, application/octet-stream)
2005-06-09 18:28 EDT, Stefan Xenos CLA
no flags Details
possible fix (4.64 KB, patch)
2005-06-14 21:32 EDT, Stefan Xenos CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tod Creasey CLA 2005-06-08 12:41:33 EDT
I ran the Resize tests on 0608 and on M6 and the Resize time for the resource
perspective has degraded from 473ms to  575ms on Windows. We are not getting
this issue on Linux.
Comment 1 Tod Creasey CLA 2005-06-08 12:55:58 EDT
Times are about the same in RC1 so it appears to have been a change between M7
(sorry it was M7) and RC1.
Comment 2 Tod Creasey CLA 2005-06-08 13:38:57 EDT
Here are the RC1 and M7 numbers. I tried using M7 SWT and RC1 UI and there was
no difference in the performance numbers so the problem was introduced by us.

RC1 numbers
GLOBAL UI - Workbench Window Resize
----- Perspective org.eclipse.ui.resourcePerspective setSize
Perspective org.eclipse.ui.resourcePerspective setSize: setUp...
Scenario 'org.eclipse.ui.tests.performance.layout.ResizeTest#Perspective
org.eclipse.ui.resourcePerspective setSize()' (average over 30 samples):
  Used Java Heap: -8268
  Working Set: 819
  Committed: 23.33K
  Working Set Peak: 819
  Elapsed Process: 552 ms
  Kernel time: 355 ms
  Page Faults: 0
  CPU Time: 505 ms
  GDI Objects: 0

M7 numbers

Perspective org.eclipse.ui.resourcePerspective setSize: setUp...
GLOBAL UI - Workbench Window Resize
Scenario 'org.eclipse.ui.tests.performance.layout.ResizeTest#Perspective
org.eclipse.ui.resourcePerspective setSize()' (average over 30 samples):
  Used Java Heap: 16.88K
  Working Set: 1.87K
  Committed: 2.27K
  Working Set Peak: 1.73K
  Elapsed Process: 335 ms
  Kernel time: 228 ms
  Page Faults: 0
  CPU Time: 334 ms
  GDI Objects: 0
Comment 3 Tod Creasey CLA 2005-06-08 15:07:53 EDT
The problem is somewhere in the IDE plugin. Here are the times with successive
additions of the RC1 plug-ins to the M7 workbench

M7 numbers

Perspective org.eclipse.ui.resourcePerspective setSize: setUp...
GLOBAL UI - Workbench Window Resize
Scenario 'org.eclipse.ui.tests.performance.layout.ResizeTest#Perspective
org.eclipse.ui.resourcePerspective setSize()' (average over 30 samples):
  Used Java Heap: 16.88K
  Working Set: 1.87K
  Committed: 2.27K
  Working Set Peak: 1.73K
  Elapsed Process: 335 ms
  Kernel time: 228 ms
  Page Faults: 0
  CPU Time: 334 ms
  GDI Objects: 0

JFace RC1

Scenario 'org.eclipse.ui.tests.performance.layout.ResizeTest#Perspective
org.eclipse.ui.resourcePerspective setSize()' (average over 30 samples):
  Used Java Heap: 9.67K
  Working Set: -546
  Committed: 1.2K
  Working Set Peak: 409
  Elapsed Process: 306 ms
  Kernel time: 180 ms
  Page Faults: 0
  CPU Time: 281 ms
  GDI Objects: 0

JFace + WOrkbench RC1


----- Perspective org.eclipse.ui.resourcePerspective setSize
Perspective org.eclipse.ui.resourcePerspective setSize: setUp...
GLOBAL UI - Workbench Window Resize
Scenario 'org.eclipse.ui.tests.performance.layout.ResizeTest#Perspective
org.eclipse.ui.resourcePerspective setSize()' (average over 30 samples):
  Used Java Heap: 12.23K
  Working Set: 2.93K
  Committed: 2.8K
  Working Set Peak: 0
  Elapsed Process: 271 ms
  Kernel time: 165 ms
  Page Faults: 1
  CPU Time: 258 ms
  GDI Objects: 0

IDE+JFace + WOrkbench RC1

Perspective org.eclipse.ui.tests.performancePerspective2 setSize: setUp...
Scenario 'org.eclipse.ui.tests.performance.layout.ResizeTest#Perspective
org.eclipse.ui.tests.performancePerspective2 setSize()' (average over 30 samples):
  Used Java Heap: 2.34K
  Working Set: 2.4K
  Committed: 1.07K
  Working Set Peak: 2.27K
  Elapsed Process: 389 ms
  Kernel time: 226 ms
  Page Faults: 0
  CPU Time: 337 ms
  GDI Objects: 0
Comment 4 Tod Creasey CLA 2005-06-08 16:47:09 EDT
I'm not sure if it was because I have been looking at this too long but the
issue seems to be the fix to Bug 71640 in IDEWorkbenchPlugin - when I run with
the M7 version of this class the performance numbers are much better than the
RC1 version.

This seems too bizarre to affect a resize test so let me dig further.
Comment 5 Tod Creasey CLA 2005-06-08 16:56:02 EDT
I am not sure if this is just variance in the suite or not so I would keep
digging around. I think this was a red herring.

RC1 with the old IDEWorkbenchPlugin
----- Perspective org.eclipse.ui.resourcePerspective setSize
Perspective org.eclipse.ui.resourcePerspective setSize: setUp...
Scenario 'org.eclipse.ui.tests.performance.layout.ResizeTest#Perspective
org.eclipse.ui.resourcePerspective setSize()' (average over 30 samples):
  Used Java Heap: 4.62K
  Working Set: 819
  Committed: 1.07K
  Working Set Peak: 819
  Elapsed Process: 241 ms
  Kernel time: 139 ms
  Page Faults: 0
  CPU Time: 225 ms
  GDI Objects: 0

With the RC1 code

Perspective org.eclipse.ui.resourcePerspective setSize: setUp...
Scenario 'org.eclipse.ui.tests.performance.layout.ResizeTest#Perspective
org.eclipse.ui.resourcePerspective setSize()' (average over 30 samples):
  Used Java Heap: 50.7K
  Working Set: 136
  Committed: 0
  Working Set Peak: 136
  Elapsed Process: 286 ms
  Kernel time: 165 ms
  Page Faults: 0
  CPU Time: 271 ms
  GDI Objects: 0
Comment 6 Stefan Xenos CLA 2005-06-08 18:15:49 EDT
Random variance shouldn't be that high for a 300ms test. Does seem weird, though.
Comment 7 Stefan Xenos CLA 2005-06-09 15:49:23 EDT
Just tested with and without the IDEWorkbenchPlugin changes, and I don't get any
measurable difference. I agree: red herring.
Comment 8 Stefan Xenos CLA 2005-06-09 16:09:38 EDT
Created attachment 22747 [details]
Profiler output for the resource perspective resize test, code in head
Comment 9 Stefan Xenos CLA 2005-06-09 18:28:48 EDT
Created attachment 22759 [details]
Profiler output for the resize tests in m7

According to this, we're actually slightly faster. Running the test in my
profiler, it took 86ms per Shell resize on m7, and it takes 75ms now.

I'll confirm by running without the profiler, but it's starting to look like
something was interfering with the test.
Comment 10 Stefan Xenos CLA 2005-06-09 19:15:46 EDT
Running without the profiler, I get a variation of about +/-100ms when running
the resize tests repeatedly. It seems that this may just be statistical variation.
Comment 11 Stefan Xenos CLA 2005-06-14 20:01:41 EDT
Figured it out. This seems to have been caused by two changes.

1. The change in bug 58003 to PresentablePartFolder and ProxyControl. Is
responsible for most of the slowdown. 
2. Coolbars and Toolbars are slower to resize. Seems to be an SWT thing.


I'll focus on issue 1 for now. Here's what the patch did:

The change in bug 58003 fixes a clipping bug that showed up in the "new"
detached view look. When using the proxy control pattern (one control that
stands in for a target control with a different parent), it needs to clip the
bounds of the target to its own visible area. 

For example, take the following widget hierarchy:
Composite1
  Composite2
    Composite3
      Proxy
  Target

Whenever the bounds of Composite2 or Composite3 overlap the bounds of Proxy (in
display coordinates), then Target needs to be set to the intersection area.

The fix added listeners to all ancestors of the proxy up to the common ancestor,
so that changes in the clipped area would be reflected in the bounds of the
target, even when the proxy hasn't moved. The trouble is that in a single
layout, many of the ancestors will move, causing the target to move more than
necessary. 

Since the bounds of the target are a function of many widgets, the target needs
to be moved using a set of resize listeners rather than a single layout. For
this reason, SWT's support for deferred layouts doesn't fix this.

I'm describing this here mainly so that I have a place to point the next time
someone asks me why proxying controls makes the layout code more complex. 

RANT MODE: ENABLED

FYI, once you add caching things get even crazier: SWT flushes layout caches by
running up the widget hierarchy, but since Target isn't a child of Composite3
any attempt to update Target's preferred size will flush the cache in Composite1
(which is unnessary and inefficient), and will fail to flush the cache in the
proxy (which results in the wrong layout). The only way to get everything right
is to know exactly who is calling computeSize on each widget, what affects the
preferred size of each widget, and manually flush all of the former whenever any
of the latter occur... and even with complete knowlegde of the entire widget
hierarchy, that's REALLY tough to get right.

SWT really needs to address this issue or we're never going to be able to
maintain this code.

RANT MODE: DISABLED


I'm going to investigate a way to defer updates to the target control as though
it were being managed by a layout.
Comment 12 Stefan Xenos CLA 2005-06-14 21:32:54 EDT
Created attachment 23123 [details]
possible fix

patch for o.e.ui.workbench
Comment 13 Stefan Xenos CLA 2005-06-14 21:48:26 EDT
With this patch, I'm still seeing runtimes about 3% slower than m7... but I
believe this is good enough that the red X's should go away. Please consider for
RC2... (and someone should confirm my benchmarks).
Comment 14 Michael Van Meekeren CLA 2005-06-16 14:37:03 EDT
Steve we introduced a slow down here and want to fix this.  We thought that the
SWT toolbar issue was the only performance issue but there were two.  

Can you agree with us fixing up some layout code here to be more optimal.  I've
looked at this patch and am going to suggest perhaps an approach where we revert
to previous behaviour.
Comment 15 Veronika Irvine CLA 2005-06-16 17:38:27 EDT
+1 for reverting ProxyControl and PresentablePartFolder to previous version of
each without clipping code.
Comment 16 Michael Van Meekeren CLA 2005-06-16 17:39:21 EDT
leaving this bug open and not marked RC3 as the code patch here is better
according to SX but reverting is lower risk for 3.1
Comment 17 Michael Van Meekeren CLA 2005-06-16 17:40:59 EDT
ccing doug as code has been committed
Comment 18 Steve Northover CLA 2005-06-16 18:15:35 EDT
Veronika has my approval to approve patch.  My time was sucked away by a 
useless meeting ...
Comment 19 Douglas Pollock CLA 2005-06-16 18:50:33 EDT
If you are committing code changes for 3.1 RC3 and wish a bug to stay open after
3.2, it would be nice if you could open a second bug.  See Bug 97783 comment #31.
Comment 20 Michael Van Meekeren CLA 2005-06-16 22:33:39 EDT
Agree doug.  Logged bug 100502 
Comment 21 Douglas Pollock CLA 2005-06-17 15:00:59 EDT
In N20050616-0010 (before patch), the following happened:
    3GHz machine, 245ms, 10.3% slower than 3.0
    2GHz machine, 282ms, 12.3% slower than 3.0

In I20050617-0010 (after patch), the following happened:
    3GHz machine, 244ms, 9.9% slower than 3.0
    2GHz machine, 276ms, 9.9% slower than 3.0
Comment 22 Stefan Xenos CLA 2005-06-17 16:07:52 EDT
Looks as though this isn't fixed. Reopening.
Comment 23 Michael Van Meekeren CLA 2005-06-17 16:08:12 EDT
This fix does not seem to be having the impact we had expected.  However it is
reasonable to keep the code in as it was our code pre-RC1 and the particular
need for the RC1 version (i.e. a fix for layout issues in a new presentation for
detached views) is not required as we no longer use this presentation.  

Marking this bug as 3.2 the tests have been slower than 3.0 all along, this
should be investigated however it is not a significant difference.
Comment 24 Paul Webster CLA 2006-09-28 15:15:04 EDT
Is this still a problem in 3.3?

PW
Comment 25 Denis Roy CLA 2007-06-22 09:33:00 EDT
Changes requested on bug 193523
Comment 26 Eclipse Webmaster CLA 2019-09-06 16:09:15 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.
Comment 27 Lars Vogel CLA 2019-11-06 04:42:08 EST
Paul, in case the latest performance do not show this as an issue, can you please close this bug?