<Swing Dev> [9] Review request for 8169922 SwingMark/TextArea: 2-7% regression on Linux, Mac, Windows in 9-b143

[Phil Race]

I notice that there is a variable that was already there (not one you 
added this time) also mis-spelled : isFPMethodOverriden I'd prefer that 
be fixed in this push but I don't need to see an updated webrev for 
that. Comments about threading noted. +1 -phil.



On 12/20/2016 12:52 PM, Alexandr Scherbatiy wrote:
> On 12/16/2016 2:36 AM, Phil Race wrote:
>> I just noticed that you spell the new methods with a single d :eg : 
>> getFPMethodOverriden
>>
>> Like the variables that were there before it should be 
>> getFPMethodOverridden
>> and checkFPMethodOverridden
>    Here is the updated fix where the typo is updated:
> http://cr.openjdk.java.net/~alexsch/8169922/webrev.02
>
>>
>> Also, although rather unlikely to be a problem in practice, since there
>> is nothing in the swing rules to prevent constructing a Swing 
>> component on
>> a random thread, before adding it to the hierarchy, and the shared 
>> Map is populated
>> at construction time, it seems like we need a ConcurrentHashMap ..
>   It seems that it could be possible that a constructor can schedule 
> to run some actions calling SwingUtilities.invokeLater().
>   In this case both threads (the thread where a Swing component is 
> constructor and EDT) can simultaneously start the object fields updating.
>   There is  still no guarantee that all works as expected if a Swing 
> component is constructed in non-EDT.
>
>   Thanks,
>   Alexandr.
>
>>
>> -phil.
>>
>> On 12/15/2016 09:06 AM, Alexandr Scherbatiy wrote:
>>>
>>> Hello,
>>>
>>> Could you review the updated fix:
>>> http://cr.openjdk.java.net/~alexsch/8169922/webrev.01
>>>
>>> - the HashMap is stored in SoftRefence
>>> - the typo in getMethodArguments() is fixed
>>> - classes FPMethodItem and FPMethodArgs are defined in the PlainView 
>>> class.
>>>
>>> Thanks,
>>> Alexandr.
>>>
>>> On 12/14/2016 8:30 PM, Sergey Bylokhov wrote:
>>>> Hi, Alexander.
>>>> Should not the storage be based on soft references? In the current 
>>>> solution we will store the references to the checked classes 
>>>> forever in the static map.
>>>>
>>>>> 13 дек. 2016 г., в 18:41, Alexandr Scherbatiy 
>>>>> <alexandr.scherbatiy at oracle.com> написал(а):
>>>>>
>>>>>
>>>>> Hello,
>>>>>
>>>>> Could you review the fix:
>>>>>   bug: https://bugs.openjdk.java.net/browse/JDK-8169922
>>>>>   webrev: http://cr.openjdk.java.net/~alexsch/8169922/webrev.00
>>>>>
>>>>>   The fix JDK-8156217 checks presence of the overridden methods 
>>>>> with floating point arguments.
>>>>>   The proposed fix caches results of found overridden methods with 
>>>>> floating point arguments.
>>>>>
>>>>>   I run the SwingSet2 with the custom test which intensively 
>>>>> creates JTextField, JPasswordField, JTextArea, and JEditorPane.
>>>>>
>>>>>   The results of the test running with the following JDK are:
>>>>>   1. Methods with int arguments are always called without the 
>>>>> methods overridden check: 45822 // before the fix JDK-8156217
>>>>>   2. Methods with floating point arguments are always called 
>>>>> without the methods overridden check: 46175
>>>>>     performance decreasing: 100 * (46175 - 45822) / 45822 = 0.77%
>>>>>   3. Methods with floating point arguments are always called with 
>>>>> the methods overridden check: 48836 // fix JDK-8156217
>>>>>     performance decreasing: 100 * (48836 - 45822) / 45822 = 6.58%
>>>>>   4. Methods with floating point arguments are always called and 
>>>>> the methods overridden checks are cached: 46592 // current fix
>>>>>     performance decreasing: 100 * (46592 - 45822) / 45822 = 1.68%
>>>>>
>>>>> Thanks,
>>>>> Alexandr.
>>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20161220/30da628b/attachment.html>