Re: Performance patch for DateFormatSymbols.getZoneIndex(String)

2012-04-24 Thread Charles Lee
Hi Deven, The patch has been committed @ Changeset: 2c35304e885a Author:youdwei Date: 2012-04-24 21:06 +0800 URL:http://hg.openjdk.java.net/jdk8/tl/jdk/rev/2c35304e885a 7163865: Performance improvement for DateFormatSymbols.getZoneIndex(String) Reviewed-by: okutsu Please verify it.

Re: Performance patch for DateFormatSymbols.getZoneIndex(String)

2012-04-24 Thread Masayoshi Okutsu
Hi Charles, Yes, it's jdk8-tl. Thanks, Masayoshi On 4/23/2012 6:01 PM, Charles Lee wrote: Hi Masayoshi, I'd like to sponsor this patch. But I did not figure out which repos the patch should go to. Is it jdk8-tl repository? On 04/23/2012 04:54 PM, Masayoshi Okutsu wrote: Looks good! Masay

Re: Performance patch for DateFormatSymbols.getZoneIndex(String)

2012-04-23 Thread Charles Lee
Hi Masayoshi, I'd like to sponsor this patch. But I did not figure out which repos the patch should go to. Is it jdk8-tl repository? On 04/23/2012 04:54 PM, Masayoshi Okutsu wrote: Looks good! Masayoshi On 4/23/2012 5:51 PM, Deven You wrote: Hi Masayoshi, Thanks for your reminder. I have

Re: Performance patch for DateFormatSymbols.getZoneIndex(String)

2012-04-23 Thread Masayoshi Okutsu
Looks good! Masayoshi On 4/23/2012 5:51 PM, Deven You wrote: Hi Masayoshi, Thanks for your reminder. I have updated the webrev[1]. Please review it. [1] http://cr.openjdk.java.net/~littlee/OJDK-548/webrev.03 Thanks a lot! On 04/23/

Re: Performance patch for DateFormatSymbols.getZoneIndex(String)

2012-04-23 Thread Deven You
Hi Masayoshi, Thanks for your reminder. I have updated the webrev[1]. Please review it. [1] http://cr.openjdk.java.net/~littlee/OJDK-548/webrev.03 Thanks a lot! On 04/23/2012 01:19 PM, Masayoshi Okutsu wrote: Hi Deven, As Yoshito no

Re: Performance patch for DateFormatSymbols.getZoneIndex(String)

2012-04-22 Thread Masayoshi Okutsu
Hi Deven, As Yoshito noted, lastZoneIndex needs to be transient because DateFormatSymbols is serializable. Otherwise, the change looks good to me. Thanks, Masayoshi On 4/20/2012 11:56 AM, Deven You wrote: Yoshito, It is good to me for your suggestion. So Masayoshi, what is your opinion, i

Re: Performance patch for DateFormatSymbols.getZoneIndex(String)

2012-04-19 Thread Deven You
Yoshito, It is good to me for your suggestion. So Masayoshi, what is your opinion, is cache the last zone index better than cache recent zone ID and zone indexes, do you have any concern about using last zone index? [1] The updated version of this patch http://cr.openjdk.java.net/~youdwei/D

Re: Performance patch for DateFormatSymbols.getZoneIndex(String)

2012-04-11 Thread Yoshito Umaoka
Deven, If last time zone ID is cached along with the previously resolved index, and getZoneIndex does not check the contents of array, it might not work when zone strings are updated (setZoneStrings). So, to make this change work properly, you have to reset lastZoneID/zoneIndex in setZoneStri

Re: Performance patch for DateFormatSymbols.getZoneIndex(String)

2012-04-11 Thread Deven You
I think Yoshito's suggestion make sense, since the getZoneIndex is an internal method, if there is no manual setting to change the timezone, The timezone ID won't be changed for one instance of SimpleDateFormt. I updated the patch webrev[1] for this suggestion and test the GetZoneIndexTest.jav

Re: Performance patch for DateFormatSymbols.getZoneIndex(String)

2012-04-10 Thread Yoshito Umaoka
I'm wondering if caching index matched last time (per DateFormatSymbols instance) is sufficient, instead of keeping multiple results in a hash map. I guess majority of use cases repeatedly refer the index of the zone used by SimpleDateFormat. -Yoshito On 4/9/2012 10:46 AM, Masayoshi Okutsu wro

Re: Performance patch for DateFormatSymbols.getZoneIndex(String)

2012-04-09 Thread Masayoshi Okutsu
Hi Deven, Here are my comments on the proposed changes. - zoneIndexCache should be an instance field because WeakHashMap isn't thread-safe and the order of IDs in zoneStrings differs in each DateFormatSymbols. - equals shouldn't be replaced with equalsIgnoreCase because time zone IDs are (s

Performance patch for DateFormatSymbols.getZoneIndex(String)

2012-04-08 Thread Deven You
Hi i18n-devs, The getZoneIndex() method is expensive and it's performance depends on which timezone you're in. This is because the code steps through a list of timezones until it finds the current one. This is happening over and over again. An idea would be to either cache it or rewrite the wa