On Tue, 11 Mar 2025 19:04:39 GMT, Chen Liang <li...@openjdk.org> wrote:
>> src/java.base/share/classes/java/lang/StableValue.java line 344:
>>
>>> 342: * {@linkplain java.lang.ref##reachability reachable} stable values
>>> will hold their set
>>> 343: * content perpetually.
>>> 344: * <p>
>>
>> Should the original functions / mappers (for stable functions and
>> collections) also stay reachable? Kotlin's
>> [`Lazy`](https://kotlinlang.org/api/core/kotlin-stdlib/kotlin/-lazy/) [nulls
>> out](https://github.com/JetBrains/kotlin/blob/c6f337283d59fcede75954eebaa589ad1b479aea/libraries/stdlib/jvm/src/kotlin/util/LazyJVM.kt#L70-L89)
>> the initializer function when it's no longer needed.
>
> The nulling out is only safe if the write to the value is visible when a
> nulled-out function is visible. I think SV can ensure this, but an
> implementation can easily go wrong trying to do this. (Also `orElseSet` does
> not NPE if the incoming supplier is null but the value is bound)
This is something we experimented with a bit in the past. It isn't easy to do
in the general case. There are pros (the function and its resources that can be
collected) and cons (e.g., mutability, visibility, complexity, etc.) with this.
>> src/java.base/share/classes/jdk/internal/lang/stable/StableValueImpl.java
>> line 141:
>>
>>> 139: ? "(this StableValue)"
>>> 140: : "StableValue" + renderWrapped(t);
>>> 141: }
>>
>> Are deeper cycles of concern? I was thinking of this:
>>
>> var a = StableValue.of();
>> var b = StableValue.of();
>> a.trySet(b);
>> b.trySet(a);
>> System.out.println(a);
>>
>>
>> This would solve deeper cycles for `StableValueImpl`:
>>
>> @Override
>> public String toString() {
>> final StringBuilder sb = new StringBuilder("StableValue");
>> int depth = 0;
>> Object t = value;
>> while (t instanceof StableValueImpl<?> s) {
>> if (s == this) {
>> t = "(this StableValue)";
>> break;
>> }
>> sb.append("[StableValue");
>> depth++;
>> t = s.value;
>> }
>> sb.append(renderWrapped(t));
>> while (depth-- > 0) sb.append(']');
>> return sb.toString();
>> }
>>
>> This might also apply to stable functions and collections, I haven't thought
>> it through for them.
>
> I think the default Object.toString impl is better here - the type
> `StableValue` shouldn't really be exposed in a user API endpoint and is just
> a utility for the users. No need to bikeshed on this mostly useless
> functionality.
The `toString()` function for stable value is inspired by `Optional` and some
of the collections.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r1990919074
PR Review Comment: https://git.openjdk.org/jdk/pull/23972#discussion_r1988637468
