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.addepa...@oracle.com>; Philip Race
<philip.r...@oracle.com>
*Cc:* swing-dev@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.addepa...@oracle.com>
<mailto:krishna.addepa...@oracle.com>
*Cc:* swing-dev@openjdk.java.net
<mailto:swing-dev@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
