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

[Chen Liang]

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

>> 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)
>
> Can I submit a PR to make this improvement?

@wenshao I agree with your proposal. Also for this part:

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);
    }
}


I recommend a rewrite:


ZoneOffset result = MINUTES_15_CACHE.getPlain(cacheIndex);
if (result == null) {
    result = new ZoneOffset(totalSeconds);
    ZoneOffset existing = MINUTES_15_CACHE.compareAndExchange(cacheIndex, null, result);
    return existing == null ? result : existing;
}


The `getPlain` is safe because `ZoneOffset` is thread safe, so you can use the object when you can observe a `ZoneOffset` object reference.  Also `compareAndExchange` avoids extra operations if we failed to racily set the computed `ZoneOffset`.

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

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