On Thu, 1 Jul 2021 22:17:27 GMT, David Holmes <dhol...@openjdk.org> wrote:

>> Please see the JBS issue for more details, but basically we have 8 different 
>> kinds of internal VM JavaThreads (grouping the three types of CompilerThread 
>> together) that all basically duplicated the logic for initializing 
>> (preparing is the term we use for user-defined JavaThreads) and starting the 
>> new thread. This common code can be factored out into static helpers in 
>> JavaThread.
>> 
>> This change does not look at the way the java.lang.Thread instance is 
>> created - that will be handled by a separate RFE.
>> 
>> The semantics of the changes are not identical, but I don't believe there is 
>> any observable change in behaviour. The scope of holding the Threads_lock 
>> has been reduced, and we now create the JavaThread instances ("new 
>> XXXThread(...)") outside of the lock. As far as I can see nothing in the 
>> construction process needs to happen under the Threads_lock.
>> 
>> A few of the threads use a static `_instance` field to hold a reference to 
>> the create JavaThread. This proved very difficult to handle, as logically 
>> the field would need to be updated in the middle of the new factored-out 
>> method: after setting all the fields but before releasing the newly started 
>> thread. I eventually realized that in all but one case those `_instance` 
>> fields are never used and so could be deleted. The one case remaining does 
>> not need to be set as I just described, but can be set after the thread has 
>> started, as the new thread does not examine it (arguably its existence is 
>> unnecessary).
>> 
>> The trickiest changes related to the CompilerThreads, so they need 
>> particular scrutiny.
>> 
>> Testing: tiers 1-3
>> 
>> Thanks,
>> David
>
> David Holmes has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   Restore null check to appease static analysis.
>   Add assertion on osthread()

Marked as reviewed by kvn (Reviewer).

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

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

Reply via email to