On Thu, 9 Feb 2023 13:46:14 GMT, Per Minborg <pminb...@openjdk.org> wrote:
>> `ZoneOffset` instances are cached by the `ZoneOffset` class itself for >> values in the range [-18h, 18h] for each second that is on an even quarter >> of an hour (i.e. at most 2*18*4+1 = 145 values). >> >> Instead of using a `ConcurrentHashMap` for caching instanced, we could >> instead use an `AtomicReferenceArray` with direct slot value access for said >> even seconds. This will improve performance and reduce the number of object >> even though the backing array will go from an initial 32 in the CHM to an >> initial/final 145 in the ARA. The CHM will contain much more objects and >> array slots for typical numbers of entries in the cache and will compute >> hash/bucket/collision on the hot code path for each cache access. > > Per Minborg has updated the pull request incrementally with three additional > commits since the last revision: > > - Remove unused setup method > - Rename method in test > - Add copyright header src/java.base/share/classes/java/time/ZoneOffset.java line 430: > 428: public static ZoneOffset ofTotalSeconds(int totalSeconds) { > 429: final class Holder { > 430: private static final IntFunction<ZoneOffset> > ZONE_OFFSET_MAPPER = new ZoneOffsetMapper(); Can't the ZoneOffsetMapper itself serve as a holder class? so we move this singleton into ZoneOffsetMapper itself. test/jdk/jdk/internal/util/LazyReferenceArray/BasicLazyReferenceArrayTest.java line 107: > 105: > 106: private static IntFunction<Integer> intIdentity() { > 107: return i -> i; Can't this be `Integer::valueOf`? ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/12346#discussion_r1142516460 PR Review Comment: https://git.openjdk.org/jdk/pull/12346#discussion_r1142520869