<i18n dev> RFR: 8345668: ZoneOffset.ofTotalSeconds performance regression [v3]

Naoto Sato naoto at openjdk.org
Fri Dec 20 23:51:36 UTC 2024


On Fri, 20 Dec 2024 23:07:47 GMT, Shaojin Wen <swen at openjdk.org> wrote:

>> src/java.base/share/classes/java/time/ZoneOffset.java line 428:
>> 
>>> 426:         if (totalSeconds % (15 * SECONDS_PER_MINUTE) == 0) {
>>> 427:             Integer totalSecs = totalSeconds;
>>> 428:             ZoneOffset result = SECONDS_CACHE.get(totalSecs);
>> 
>> Here, each call may allocate an Integer object. The maximum number of ZoneOffsets that need to be cached here is only 148. Using AtomicReferenceArray is better than AtomicConcurrentHashMap.
>
> For example:
> 
> static final AtomicReferenceArray<ZoneOffset> MINUTES_15_CACHE = new AtomicReferenceArray<>(37 * 4);
> 
>     public static ZoneOffset ofTotalSeconds(int totalSeconds) {
>         // ...
>         int minutes15Rem = totalSeconds / (15 * SECONDS_PER_MINUTE);
>         if (totalSeconds - minutes15Rem * 15 * SECONDS_PER_MINUTE == 0) {
>             int cacheIndex = minutes15Rem + 18 * 4;
>             ZoneOffset result = MINUTES_15_CACHE.get(cacheIndex);
>             if (result == null) {
>                 result = new ZoneOffset(totalSeconds);
>                 if (!MINUTES_15_CACHE.compareAndSet(cacheIndex, null, result)) {
>                     result = MINUTES_15_CACHE.get(minutes15Rem);
>                 }
>             }
>             return result;
>         }
>        // ...
>     }

Hi Shaojin,
Thanks for the suggestion, but I am not planning to improve the code more than backing out the offending fix at this time. (btw, cache size would be 149 as 18:00 and -18:00 are inclusive)

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/22854#discussion_r1894505406


More information about the i18n-dev mailing list