RFR: 8305425: Thread.isAlive0 doesn't need to call into the VM [v6]

ExE Boss duke at openjdk.org
Wed Apr 5 04:12:18 UTC 2023


On Wed, 5 Apr 2023 02:30:15 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> We have the strange situation where calling `t.isAlive()` on a `java.lang.Thread` `t`, will call into the VM (via `alive()` then `isAlive0()`) where the VM then examines the `eetop` field of `t` to extract its `JavaThread` pointer and compare it to null. We can simply read `eetop` directly in `Thread.alive()`:
>> 
>> boolean alive() {
>>   return eetop != 0;
>> } 
>> 
>> I also updated a comment in relation to `eetop`.
>> 
>> Testing: tiers 1-3
>> 
>> Thanks
>
> David Holmes has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Switch from using synchronized to using a volatile eetop field
>  - Added Shipilev's test (with a small addition)

src/hotspot/share/runtime/javaThread.cpp line 743:

> 741:   // Clear the native thread instance - this makes isAlive return false and allows the join()
> 742:   // to complete once we've done the notify_all below. Needs a release() to obey Java Memory Model
> 743:   // requirements.

This might be more readable:
Suggestion:

  // to complete once we've done the notify_all below.
  // Needs a release() to obey Java Memory Model requirements.

src/java.base/share/classes/java/lang/Thread.java line 235:

> 233:        and reset to zero when the thread terminates. A non-zero value indicates this thread
> 234:        isAlive().
> 235:     */

Maybe JavaDocify this?
Suggestion:

    /**
     * Reserved for exclusive use by the JVM. Cannot be moved to the FieldHolder
     * as it needs to be set by the VM before executing the constructor that
     * will create the FieldHolder. The historically named {@code eetop} holds
     * the address of the underlying VM JavaThread, and is set to non-zero when
     * the thread is started, and reset to zero when the thread terminates.
     * A non-zero value indicates this thread {@link #isAlive()}.
     */

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13287#discussion_r1157996691
PR Review Comment: https://git.openjdk.org/jdk/pull/13287#discussion_r1157998171



More information about the build-dev mailing list