RFR: 8356080: Address post-integration comments for Stable Values

[Shaojin Wen]

On Fri, 2 May 2025 12:13:39 GMT, Per Minborg <pminborg at openjdk.org> wrote:

> This PR proposes to address comments in the initial PR for Stable Values, which were deferred until after integration.
> 
> Unfortunately, this PR shows the total commit history for stable values.

src/java.base/share/classes/java/util/ImmutableCollections.java line 895:

> 893:             public List<E> subList(int fromIndex, int toIndex) {
> 894:                 int size = size();
> 895:                 subListRangeCheck(fromIndex, toIndex, size);

Suggestion:

                subListRangeCheck(fromIndex, toIndex, size());

size is only used once, can it be simplified like this?

src/java.base/share/classes/java/util/ImmutableCollections.java line 905:

> 903:                 // Todo: Provide a copy free solution
> 904:                 final StableValueImpl<E>[] reversed = Arrays.copyOfRange(delegates, this.offset, this.size - offset);
> 905:                 return StableUtil.renderElements(this, "StableCollection", reversed);

Suggestion:

                return StableUtil.renderElements(this, "StableCollection", 
                                                 Arrays.copyOfRange(deepRoot(root).delegates, this.offset, this.size - offset));

The three local variables `deepRoot/delegates/reversed` are only used once. Can they be simplified like this?

src/java.base/share/classes/java/util/ImmutableCollections.java line 932:

> 930:                 final StableValueImpl<E>[] reversed = ArraysSupport.reverse(
> 931:                         Arrays.copyOf(delegates, delegates.length));
> 932:                 return StableUtil.renderElements(this, "Collection", reversed);

Suggestion:

                final StableValueImpl<E>[] delegates = deepRoot(base).delegates;
                // Todo: Provide a copy free solution
                return StableUtil.renderElements(this, "Collection", ArraysSupport.reverse(Arrays.copyOf(delegates, delegates.length)));


Same as above, simplifying the code by removing local variables that are only used once

src/java.base/share/classes/jdk/internal/lang/stable/StableSupplier.java line 62:

> 60:     public String toString() {
> 61:         final Object t = delegate.wrappedContentsAcquire();
> 62:         return t == this ? "(this StableSupplier)" : StableValueImpl.renderWrapped(t);

Suggestion:

        return delegate.wrappedContentsAcquire() == this 
               ? "(this StableSupplier)" 
               : StableValueImpl.renderWrapped(t);

Same as above, simplifying the code by removing local variables that are only used once

src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java line 54:

> 52:     // Unsafe offsets for direct field access
> 53: 
> 54:     private static final long CONTENT_OFFSET =

It should be CONTENTS_OFFSET, right?

src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java line 108:

> 106:         if (t == null) {
> 107:             throw new NoSuchElementException("No contents set");
> 108:         }

Suggestion:

        if (wrappedContentsAcquire() == null) {
            throw new NoSuchElementException("No contents set");
        }

Same as above, simplifying the code by removing local variables that are only used once

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25004#discussion_r2071635586
PR Review Comment: https://git.openjdk.org/jdk/pull/25004#discussion_r2071642599
PR Review Comment: https://git.openjdk.org/jdk/pull/25004#discussion_r2071648603
PR Review Comment: https://git.openjdk.org/jdk/pull/25004#discussion_r2071651874
PR Review Comment: https://git.openjdk.org/jdk/pull/25004#discussion_r2071654313
PR Review Comment: https://git.openjdk.org/jdk/pull/25004#discussion_r2071656349