RFR: JDK-8286666: JEP 429: Implementation of Scoped Values (Incubator) [v25]

[Paul Sandoz]

On Wed, 23 Nov 2022 18:39:19 GMT, Andrew Haley <aph at openjdk.org> wrote:

>> JEP 429 implementation.
>
> Andrew Haley has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 52 commits:
> 
>  - Merge master
>  - javadoc
>  - Feedback from reviewers
>  - Update src/hotspot/share/classfile/vmSymbols.hpp
>  - Merge branch 'JDK-8286666' of https://github.com/theRealAph/jdk into JDK-8286666
>  - Update src/java.base/share/classes/java/lang/VirtualThread.java
>    
>    Co-authored-by: Alan Bateman <Alan.Bateman at oracle.com>
>  - Update src/java.base/share/classes/java/lang/Thread.java
>    
>    Co-authored-by: ExE Boss <3889017+ExE-Boss at users.noreply.github.com>
>  - Update src/hotspot/cpu/aarch64/aarch64.ad
>    
>    Co-authored-by: ExE Boss <3889017+ExE-Boss at users.noreply.github.com>
>  - Feedback from reviewers
>  - Remove incorrect assertion.
>  - ... and 42 more: https://git.openjdk.org/jdk/compare/2afb4c33...30f150e1

Looks good (just some last minor comments). I did not focus on the tests, nor too closely at all of the HotSpot changes.

Something for future investigation perhaps (if not already thought about): consider using a persistent map, a Hash Array Mapped Trie (HAMT), for storing scoped keys and values, which could potentially remove the need for the cache of replace the cache when many values are in scope. The HAMT's structural sharing properties, wide-branching factor, and `Integer.bitCount` being intrinsic all make for an efficient implementation.

src/hotspot/share/classfile/vmSymbols.hpp line 401:

> 399:   template(daemon_name,                               "daemon")                                   \
> 400:   template(run_method_name,                           "run")                                      \
> 401:   template(call_method_name,                          "call")                                     \

Is this used?

src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/ScopedValue.java line 209:

> 207:         final int bitmask;
> 208: 
> 209:         private static final Object NIL = new Object();

Suggestion:

        static final Object NO_VALUE = new Object();

src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/ScopedValue.java line 212:

> 210: 
> 211:         static final Snapshot EMPTY_SNAPSHOT = new Snapshot();
> 212:         Snapshot(Carrier bindings, Snapshot prev) {

Suggestion:

        static final Snapshot EMPTY_SNAPSHOT = new Snapshot();
        
        Snapshot(Carrier bindings, Snapshot prev) {

src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/ScopedValue.java line 218:

> 216:         }
> 217: 
> 218:         protected Snapshot() {

Suggestion:

        Snapshot() {

src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/ScopedValue.java line 464:

> 462:      * Calls a value-returning operation with a {@code ScopedValue} bound to a value
> 463:      * in the current thread. When the operation completes (normally or with an
> 464:      * exception), the {@code ScopedValue} will revert to being unbound, or rervert to

Suggestion:

     * exception), the {@code ScopedValue} will revert to being unbound, or revert to

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

Marked as reviewed by psandoz (Reviewer).

PR: https://git.openjdk.org/jdk/pull/10952