<AWT Dev> [9] Review Request: 8071306 GUI perfomance are very slow compared java 1.6.0_45

[Anton V. Tarasov]

Hi Sergey,

On 09.05.2015 3:41, Sergey Bylokhov wrote:
> Hi, Anton.
> On 08.05.15 12:23, Anton V. Tarasov wrote:
>>
>> 1314     /**
>> 1315      * Determines the bounds which will be displayed on the screen.
>> 1316      *
>> 1317      * @return the visible part of bounds in the coordinate space of this comp
>> 1318      */
>> 1319     private Rectangle getRecursivelyVisibleBounds() {
>>
>> Could you please clarify the comment, it's not quite clear from the first glance. Something like:
>>
>> "the bounds of a visible part of the component relative to..."
> The patch updated:
> http://cr.openjdk.java.net/~serb/8071306/webrev.04

+     * Determines the bounds of a visible part of the component relative to its
+     * parents.


Did you mean "to its parent"? (If so, I don't mind fixing it in a commit changeset only).

>>
>> 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.

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20150512/f4b7339a/attachment-0001.html>