Hi Sergey,
the fix looks good to me too.
--
Thanks,
Alexander.
On 05/21/2015 04:29 PM, Sergey Bylokhov wrote:
Hello,
Any volunteers to be a second reviewer?
On 18.05.15 12:03, Anton V. Tarasov wrote:
On 15.05.2015 1:57, Sergey Bylokhov wrote:
Hi, Anton.
+ * Determines the bounds of a visible part of the component relative to its
+ * parents.
Did you mean "to its parent"?
Yep, new version:
http://cr.openjdk.java.net/~serb/8071306/webrev.05/
Looks good to me.
Regards,
Anton.
2.
100 * The components in this container.
101 * @see #add
102 * @see #getComponents
103 */
104 private java.util.List<Component> component = new
ArrayList<>();
May be it's worth to rename the field? The "component" name is odd...
I suppose it wasn't changed, because this name is used in the
serialization for a long time. Plus there are a bunch of the
similar vars like: tmpComponent etc. I can do it later for jdk9 only.
Ok, thanks. It's up to you.
Regards,
Anton.
Regards,
Anton.
On 07.05.2015 3:39, Sergey Bylokhov wrote:
Hello.
Please review the fix for a jdk9. I plan to backport it to jdk8u60.
Description.
An UIworks really slowly, when an application has a lot of
components in one container, and these components should be
disabled one by one.
The reason is the next sequence of methods calls:
Component.setEnabled->updateCursorImmediately()-> some cursor
related
staff->GlobalCursorManager._updateCursor->Container.findComponentAt()-iteration
over all components in the container.....-> twice....
You can imagine how it works in case of 10000 components in the
container.
Note that in the bug report described difference jdk6 vs jdk8 ->
1sec vs 6 sec. This was caused by the two fixes, one of which
adds checkTreeLock() and in another one a simple array of
components was replaced by the ArrayList. Since code was added
to the really hot method we got so big slowdown.
To fix the problem I suggest two different approaches:
- Container.java: Fix a general case, by eliminating a second
iteration in the hot loop.
- Component.java: Totally eliminates cursor machinery, if
component cannot affect current cursor.
Some speedup measurements on my local system:
- Simple removing of checkTreeLock() will partly solve a
regression reported by the user(12 sec -> 5 sec).
- Changes in the loop will speedup the code in the worse
case(5->2 sec)
- The changes in the Component.java will change the performance
from 2 sec to 100< ms
Test case which was added was improved from 10 seconds to 80 ms.
JMH test:
http://cr.openjdk.java.net/~serb/8071306/SetEnabledPerformanceTest.java
Fixedversion:
Benchmark Mode Cnt Score Error Units
SetEnabledPerformanceTest.testContains thrpt 5 301300,813 ±
17338,045 ops/ms
SetEnabledPerformanceTest.testFindComponentAt thrpt 5
20,521 ± 0,269 ops/ms
SetEnabledPerformanceTest.testGetComponentAt thrpt 5
22,297 ± 1,264 ops/ms
SetEnabledPerformanceTest.testSetEnabled thrpt 5 711,120
± 19,837 ops/ms
Base version:
Benchmark Mode Cnt Score Error Units
SetEnabledPerformanceTest.testContains thrpt 5 299145,642 ±
2120,183 ops/ms
SetEnabledPerformanceTest.testFindComponentAt thrpt 5
1,101 ± 0,012 ops/ms
SetEnabledPerformanceTest.testGetComponentAt thrpt 5
6,792 ± 0,097 ops/ms
SetEnabledPerformanceTest.testSetEnabled thrpt 5 0,464
± 0,020 ops/ms
Bug: https://bugs.openjdk.java.net/browse/JDK-8071306
Webrev can be found at:
http://cr.openjdk.java.net/~serb/8071306/webrev.03
--
Best regards, Sergey.
--
Best regards, Sergey.
--
Best regards, Sergey.
--
Best regards, Sergey.
