RFR: 8269652: Factor out the common code for creating system j.l.Thread objects

[David Holmes]

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 <dholmes at 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
>