On Fri, 20 Dec 2024 23:59:30 GMT, Shaojin Wen <s...@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
