<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