On Tue, 22 Jun 2021 01:11:30 GMT, Guoxiong Li <g...@openjdk.org> wrote:
> Hi all,
>
> Considering the consistency of `JavaThread` and other threads, such as
> WorkerThread and CompilerThread, `JavaThread` could use a method named `cast`
> to replace the method `Thread::as_Java_thread()`. It can reduce the Thread's
> knowledge about the subtypes.
>
> This patch removes two methods, `JavaThread* Thread::as_Java_thread()` and
> `const JavaThread* Thread::as_Java_thread() const`, of the class `Thread` and
> adds two static methods, `JavaThread* cast(Thread* t)` and `const JavaThread*
> cast(const Thread* t)`, to the class `JavaThread`. Correspondingly, the code
> of the method `JavaThread::current()` need to be adjusted and many places
> where the method `Thread::as_Java_thread()` is used need to use
> `JavaThread::cast` instead.
>
> Test:
> tier1 passed locally.
>
> Thanks for taking the time to review.
>
> Best Regards,
> -- Guoxiong
Hi Guoxiong,
Thanks for picking up this enhancement request.
I wasn't sure if this would be worth the churn/disruption to the source code,
but I think it is ok and preferable to use the cast notation.
The changes look good except for one mistake flagged below.
Note you need at least two reviewers before integrating this.
Thanks,
David
src/hotspot/share/gc/z/zFuture.inline.hpp line 49:
> 47: // Wait for notification
> 48: Thread* const thread = Thread::current();
> 49: if (JavaThread::cast(thread)) {
This is wrong - we still need the is_Java_thread() query; and cast is not a
boolean operator.
-------------
Changes requested by dholmes (Reviewer).
PR: https://git.openjdk.java.net/jdk/pull/4546
