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

Reply via email to