RFR: 8269466: Factor out the common code for initializing and starting internal VM JavaThreads [v2]
David Holmes
david.holmes at oracle.com
Thu Jul 1 01:59:30 UTC 2021
Thanks for the review Patricio.
David
On 1/07/2021 1:57 am, Patricio Chilano Mateo wrote:
> On Wed, 30 Jun 2021 01:31:30 GMT, David Holmes <dholmes at 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:
>>
>> Fixed copyright years in hpp files
>
> Hi David,
>
> Changes look good to me.
>
> Thanks,
> Patricio
>
> -------------
>
> Marked as reviewed by pchilanomate (Committer).
>
> PR: https://git.openjdk.java.net/jdk/pull/4629
>
More information about the serviceability-dev
mailing list