8356080: Address post-integration comments for Stable Values

[minborg]

This PR proposes to address comments in the initial PR for Stable Values, which were deferred until after integration.

Most of the efforts in this PR are to retain "stability" as long as possible so that views of stable collections remain stable and do not evaluate on toString() for example.

Unfortunately, this PR shows the total commit history for stable values.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change requires CSR request JDK-8356081 to be approved
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issues

• JDK-8356080: Address post-integration comments for Stable Values (Bug - P4)
• JDK-8356081: Address post-integration comments for Stable Values (CSR)

Reviewers

• Chen Liang (@liach - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25004/head:pull/25004
$ git checkout pull/25004

Update a local copy of the PR:
$ git checkout pull/25004
$ git pull https://git.openjdk.org/jdk.git pull/25004/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 25004

View PR using the GUI difftool:
$ git pr show -t 25004

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25004.diff

Using Webrev

Link to Webrev Comment

[mlbridge]

Webrevs

• 03: Full - Incremental (cebd1c0a)
• 02: Full (a2826336)
• 01: Full (3eebd504)
• 00: Full (116ce16c)

[mcimadamore]

StableAPI javadoc changes look good!

[liach]

Looks good, some bikeshedding

[liach]

lgtm!

[lukellmann]

I noticed that this code will throw NoSuchElementException instead of IndexOutOfBoundsException and will swallow the message because of the try/catch in ImmutableCollections.ListItr.next/previous:

StableValue
    .list(42, _ -> {throw new IndexOutOfBoundsException("important message");})
    .listIterator(5)
    .next(); // same for previous()

[lukellmann]

Do you plan to implement this suggestion: #23972 (comment)?

[minborg]

I noticed that this code will throw NoSuchElementException instead of IndexOutOfBoundsException and will swallow the message because of the try/catch in ImmutableCollections.ListItr.next/previous:

StableValue
    .list(42, _ -> {throw new IndexOutOfBoundsException("important message");})
    .listIterator(5)
    .next(); // same for previous()

We will take a look at this under a separate issue: https://bugs.openjdk.org/browse/JDK-8356842

[liach]

The latest change set looks fine.

[minborg]

/integrate

Thank you all reviewers for reviewing this PR!

[openjdk]

Going to push as commit 066477d.
Since your change was applied there have been 36 commits pushed to the master branch:

• fa41948: 8350765: Need to pin when accessing thread container from virtual thread
• 48d2acb: 8356783: CompilerTask hot_method is redundant
• de10644: 8356631: OopHandle replacement methods should not be called on empty handles
• ... and 33 more: https://git.openjdk.org/jdk/compare/43008b42cf98466b4c4528d357bfcc2ae309770a...master

Your commit was automatically rebased without conflicts.

[openjdk]

@minborg Pushed as commit 066477d.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

[lukellmann]

Suggested change

	&& Objects.equals(getValue(), e.getValue());
	&& Objects.equals(e.getValue(), getValue());

You already integrated and it's probably not that important, but this would additionally avoid evaluation if e.getValue() throws.

[liach]

This is in the realms of microoptimization now...