<Swing Dev> [11][JDK-8197785]javax.accessibility.AccessibilityBundle will reload the ResourceBundle for every call to toDisplayString

[Semyon Sadetsky]

On 03/05/2018 10:41 AM, Krishna Addepalli wrote:

> Hi Semyon,
>
> Thank you for the review. I don’t see any reason why HashMap can’t be 
> used here. But could you clarify what you meant by “synchronization is 
> surplus here”?
>
Sure. Hashtable is synchronized. And since the locale bundle is created 
once now and then is only read there is no need to have extra thread 
access synchronization.
>
> Besides, whether we use HashMap /Hashtable, the fix for this 
> particular bug still remains.
>
> As for changing (if that is necessary), I think we should address it 
> in a separate bug?
>
It is not unrelated completely since you are fixing a performance issue. 
I suggest to fix it for one since it is easy.

--Semyon
>
> Thanks,
>
> Krishna
>
> *From:*Semyon Sadetsky
> *Sent:* Monday, March 5, 2018 10:39 PM
> *To:* Krishna Addepalli <krishna.addepalli at oracle.com>; Philip Race 
> <philip.race at oracle.com>
> *Cc:* swing-dev at openjdk.java.net
> *Subject:* Re: <Swing Dev> 
> [11][JDK-8197785]javax.accessibility.AccessibilityBundle will reload 
> the ResourceBundle for every call to toDisplayString
>
> On 03/05/2018 09:00 AM, Semyon Sadetsky wrote:
>
>     Hi Krishna,
>
>     Is there any reason to use Hashmap. It seems the synchronization
>     is surplus here and its better to use HashMap.
>
> I meant Hashtable. Sorry.
>
> --Semyon
>
>     --Semyon
>
>     On 03/05/2018 05:39 AM, Krishna Addepalli wrote:
>
>         Hi Phil
>
>         Thank you for the review.
>
>         I have checked the accessibility package, and found that
>         contains is used with a Vector of AccessibleState objects in
>         the file AccessibleStateset.
>
>         However in AccessibilityBundle, the table is used to store a
>         set of values associated with a particular locale, and going
>         by the context, I find little reason that using contains is
>         intentional here.
>
>         And regarding the testcase, the thought of making it headless
>         crossed my mind while writing it, but I was not sure if just
>         creating a Button is allowed in headless mode.
>
>         Fortunately, I modified the testcase enough that, no swing
>         widgets need to be created, and we can safely run the test in
>         headless mode.
>
>         Here is the new webrev:
>         http://cr.openjdk.java.net/~kaddepalli/8197785/webrev01/
>         <http://cr.openjdk.java.net/%7Ekaddepalli/8197785/webrev01/>
>
>         Thanks,
>
>         Krishna
>
>         *From:*Philip Race
>         *Sent:* Saturday, March 3, 2018 9:44 PM
>         *To:* Krishna Addepalli <krishna.addepalli at oracle.com>
>         <mailto:krishna.addepalli at oracle.com>
>         *Cc:* swing-dev at openjdk.java.net
>         <mailto:swing-dev at openjdk.java.net>
>         *Subject:* Re: <Swing Dev>
>         [11][JDK-8197785]javax.accessibility.AccessibilityBundle will
>         reload the ResourceBundle for every call to toDisplayString
>
>         The fix is straightforward and I've seen this bug pattern before.
>         In fact there may even have been a sweep for uses of
>         contains() to make sure it was as intended,
>         but if so it wasn't thorough enough.
>         But I'm wondering why the test extends Button and not JButton ?
>         I'm then further wondering if it could then be made headless
>         .. ie no need to create or display a Frame ?
>
>         -phil.
>
>         On 3/3/18, 6:30 AM, Krishna Addepalli wrote:
>
>             Hi All,
>
>             Please review a simple fix for JDK-8197785:
>             https://bugs.openjdk.java.net/browse/JDK-8197785
>
>             Webrev:
>             http://cr.openjdk.java.net/~kaddepalli/8197785/webrev00/
>             <http://cr.openjdk.java.net/%7Ekaddepalli/8197785/webrev00/>
>
>             As the bug description suggests,
>             AccessibleBundle.loadResourceBundle has the line
>             “table.contains” which causes it to reload the resource
>             bundle for each call.
>
>             Changing it to “table.containsKey” fixes the problem.
>
>             Thanks,
>
>             Krishna
>

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