On Wed, 2 Apr 2025 14:30:05 GMT, Viktor Klang <vkl...@openjdk.org> wrote:
>> Per Minborg has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Add info that Map#values and Map#entrySet are stable
>
> src/java.base/share/classes/java/lang/StableValue.java line 426:
>
>> 424: * {@linkplain java.lang.ref##reachability strongly
>> reachable}. Clients are
>> 425: * advised that {@linkplain java.lang.ref##reachability
>> reachable} stable values
>> 426: * will hold their set content perpetually.
>
> This reads like the contents of StableValues will never be garbage collected,
> even if the StableValue is no longer reachable. Is that intended?
I've changed the wording now so it becomes clearer that the content can be GCed
when the SV gets GCed.
> src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java
> line 191:
>
>> 189: @ForceInline
>> 190: private static Object wrap(Object t) {
>> 191: return (t == null) ? NULL_SENTINEL : t;
>
> Personal preference, but it would seem more consistent to do the same check
> in wrap as in unwrap:
>
> Suggestion:
>
> return (t != null) ? t : NULL_SENTINEL;
I've changed the unwrap method...
> test/jdk/java/lang/StableValue/StableValuesSafePublicationTest.java line 64:
>
>> 62: // fully initialized thanks to the HB properties of StableValue.
>> 63: int a;
>> 64: int b;
>
> Might be worth putting in some more fields to increase the chance of
> detecting a mismatch.
Good idea!
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2026777162
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2026775227
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r2026769897
