RFR: 8284161: Implementation of Virtual Threads (Preview) [v4]

[Alan Bateman]

On Tue, 19 Apr 2022 00:09:04 GMT, Mandy Chung <mchung at openjdk.org> wrote:

>> Alan Bateman has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Refresh
>
> src/java.base/share/classes/java/lang/ref/ReferenceQueue.java line 61:
> 
>> 59:     private final Condition notEmpty;
>> 60: 
>> 61:     void signal() { notEmpty.signalAll(); }
> 
> `signal()`, `await()` and `await(long)` methods are only used by `ReferenceQueue`.  Good to make them private.

They are overridden so can't be private.

> src/java.base/share/classes/java/lang/ref/ReferenceQueue.java line 206:
> 
>> 204:         throws IllegalArgumentException, InterruptedException {
>> 205:         if (timeout < 0) throw new IllegalArgumentException("Negative timeout value");
>> 206:         if (timeout == 0) return remove();
> 
> Nit: use the same formatting as in `NativeReferenceQueue::remove` and in this file.
> 
> 
>         if (timeout < 0)
>             throw new IllegalArgumentException("Negative timeout value");
>         if (timeout == 0)
>             return remove();

okay

> src/java.management/share/classes/java/lang/management/ThreadMXBean.java line 655:
> 
>> 653:      * synchronization control.  It might be an expensive operation.
>> 654:      * Its behavior with cycles involving virtual threads is not defined
>> 655:      * in this release.
> 
> What does the current implementation return if the virtual threads are involved in a deadlock?    The class spec says "ThreadMXBean does not support monitoring or management of virtual threads" while this javadoc says it's undefined.    I wonder if it's helpful to include `@implNote` if appropriate.

If there is cycle involving a virtual thread then the thread id of its carrier will be included in array. I think we can do better and include the thread id of the mounted thread.

> src/jdk.management/share/classes/com/sun/management/HotSpotDiagnosticMXBean.java line 138:
> 
>> 136:      *
>> 137:      * @param  outputFile the path to the file to create
>> 138:      * @param  format the format to use (TEXT_PLAIN or JSON)
> 
> It's useful to link to `ThreadDumpFormat#TEXT_PLAN` and `ThreadDumpFormat#JSON`

I think it might be better if the description is changed to "the format to use". There is already a link to the ThreadDumpFormat enum in method signature.

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

PR: https://git.openjdk.java.net/jdk/pull/8166