Community
Participate
Working Groups
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.
Times are about the same in RC1 so it appears to have been a change between M7 (sorry it was M7) and RC1.
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
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
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.
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
Random variance shouldn't be that high for a 300ms test. Does seem weird, though.
Just tested with and without the IDEWorkbenchPlugin changes, and I don't get any measurable difference. I agree: red herring.
Created attachment 22747 [details] Profiler output for the resource perspective resize test, code in head
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.
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.
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.
Created attachment 23123 [details] possible fix patch for o.e.ui.workbench
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).
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.
+1 for reverting ProxyControl and PresentablePartFolder to previous version of each without clipping code.
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
ccing doug as code has been committed
Veronika has my approval to approve patch. My time was sucked away by a useless meeting ...
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.
Agree doug. Logged bug 100502
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
Looks as though this isn't fixed. Reopening.
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.
Is this still a problem in 3.3? PW
Changes requested on bug 193523
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.
Paul, in case the latest performance do not show this as an issue, can you please close this bug?