Hi Xin,
Thanks for taking a look at this.
On 3/07/2021 1:10 pm, Xin Liu wrote:
On Fri, 2 Jul 2021 07:03:50 GMT, David Holmes <dhol...@openjdk.org> wrote:
Please review this simple refactoring to share the common code used to create
j.l.Thread instances for the internal VM JavaThreads. (Also fix a missing space
from my previous change in thread.cpp.)
It is all very straight-forward.
Compiler folk I can go one step further and remove
CompileBroker::create_thread_oop if desired.
Testing: tiers 1-3, GHA
Thanks,
David
Hi, David,
I found you wrote the same comment twice. one is in thread.hpp. the other one
is thread.cpp. Is it necessary? if we only need to keep one copy, should we
write comments in header file or impl file?
Good question- - there is no consistent style for this. I don't have a
problem commenting both to some degree.
The second thing your change is not exactly same for attachListener.cpp. Originally, it returns if
JavaCalls::construct_new_instance() has trouble, ie. `has_init_error(THREAD)` returns true. In your change,
you call "add group" but check error later. I guess call "call_special" should throw a
java exception instead of just setting its state "AL_NOT_INITIALIZED". Does it matter?
The code is the same. The CHECK_NH macro will return from
create_system_thread_object if any of the calls result in an exception
pending.
Other part of code looks good.
Thanks,
David
-----
-------------
PR: https://git.openjdk.java.net/jdk/pull/4663
