On Wed, 16 Nov 2022 16:55:24 GMT, Andrew Haley <a...@openjdk.org> wrote:
>> JEP 429 implementation.
>
> Andrew Haley has updated the pull request incrementally with two additional
> commits since the last revision:
>
> - Javadoc changes.
> - ProblemList.txt cleanup
src/java.base/share/classes/java/lang/Thread.java line 744:
> 742:
> 743: // special value to mean a new thread
> 744: this.scopedValueBindings = Thread.class;
Perhaps:
static final Object NEW_THREAD_BINDINGS = Thread.class;
...
this.scopedValueBindings = NEW_THREAD_BINDINGS;
?
src/java.base/share/classes/java/lang/Thread.java line 1614:
> 1612: }
> 1613:
> 1614: @Hidden
Should we document that the name and signature are special (same of other
relevant methods not already documented).
src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/ScopedValue.java
line 49:
> 47: *
> 48: * <p> {@code ScopedValue} defines the {@link #where(ScopedValue, Object,
> Runnable)}
> 49: * method to set the value of a {@code ScopedValue} for the bouned period
> of execution by
Suggestion:
* method to set the value of a {@code ScopedValue} for the bounded period of
execution by
src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/ScopedValue.java
line 241:
> 239: }
> 240:
> 241: static final class EmptySnapshot extends Snapshot {
We could make `Snapshot` final have a static final field?
static final Snapshot EMPTY_SNAPSHOT = new Snapshot();
src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/ScopedValue.java
line 391:
> 389: * JVM_FindScopedValueBindings().
> 390: */
> 391: private <R> R runWith(Snapshot newSnapshot, Callable<R> op)
> throws Exception {
Missing `@Hidden` and `@ForceInline` ? like with the other `runWith` method
accepting `Runnable`?
(I was gonna suggest changing the name to `callWith`, but then reread the docs
and VM code. Its convenient to have the same names.)
src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/ScopedValue.java
line 672:
> 670: return EmptySnapshot.getInstance();
> 671: }
> 672: if (bindings == null) {
Suggestion:
if (bindings == NEW_THREAD_BINDINGS) {
// This must be a new thread
return Snapshot.EMPTY_SNAPSHOT;
} else if (bindings == null) {
src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/ScopedValue.java
line 720:
> 718: // is invoked, we record the result of the lookup in this per-thread
> cache
> 719: // for fast access in future.
> 720: private static class Cache {
Make the class final and remove the qualifier on all the methods.
src/jdk.incubator.concurrent/share/classes/jdk/incubator/concurrent/ScopedValue.java
line 824:
> 822: }
> 823:
> 824: public static void invalidate() {
Is this method used?
-------------
PR: https://git.openjdk.org/jdk/pull/10952
