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

Alan Bateman alanb at openjdk.java.net
Sat Apr 16 14:26:43 UTC 2022


On Fri, 15 Apr 2022 20:39:51 GMT, Paul Sandoz <psandoz 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/jdk/internal/misc/Blocker.java line 126:
> 
>> 124:         static void endCompensatedBlock(ForkJoinPool pool, long post) {
>> 125:             try {
>> 126:                 endCompensatedBlock.invoke(pool, post);
> 
> Suggestion:
> 
>                 endCompensatedBlock.invokeExact(pool, post);

Yes, that would be better.

> src/java.base/share/classes/jdk/internal/misc/InternalLock.java line 46:
> 
>> 44:         } else {
>> 45:             CAN_USE_INTERNAL_LOCK = true;
>> 46:         }
> 
> Suggestion:
> 
>         CAN_USE_INTERNAL_LOCK = Boolean.getBoolean("jdk.io.useMonitors");

This  doesn't work for the empty string case as it would map to true rather than false. So I think we leave this as is.

> src/java.base/share/classes/jdk/internal/misc/ThreadFlock.java line 262:
> 
>> 260:      * @return the thread, started
>> 261:      * @throws IllegalStateException if this flock is shutdown or closed
>> 262:      * @throws jdk.incubator.concurrent.StructureViolationException if the current
> 
> Suggestion:
> 
>      * @throws WrongThreadException if the current thread is not the owner or a thread
>      * contained in the flock
>      * @throws jdk.incubator.concurrent.StructureViolationException if the current

Thanks, I think this slipped in when moving the SL API to the incubator module. It should be IllegalThreadStateException if the thread has already started, and WrongThreadException if not the owner (or flock).

> src/java.base/share/classes/jdk/internal/misc/ThreadTracker.java line 59:
> 
>> 57:                     && this.thread == other.thread;
>> 58:         }
>> 59:     }
> 
> Suggestion:
> 
>     private record ThreadRef(Thread thread) {
>         @Override
>         public int hashCode() {
>             return Long.hashCode(thread.threadId());
>         }
> 
>         @Override
>         public boolean equals(Object obj) {
>             return (obj instanceof ThreadRef other)
>                     && this.thread == other.thread;
>         }
>     }

Yes, this can be a record.

> src/java.base/share/classes/jdk/internal/vm/annotation/ChangesCurrentThread.java line 35:
> 
>> 33:  * disables inlining for the method to which it is applied unless the
>> 34:  * method being unlined into is also annotated ChangesCurrentThread.
>> 35: 
> 
> Suggestion:
> 
>  * method being inlined into is also annotated ChangesCurrentThread.
>  *

Thanks.

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

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


More information about the serviceability-dev mailing list