RFR (M): 8159984: Remove call to ClassLoaderDataGraph::clear_claimed_marks during the initial mark pause

Kim Barrett kim.barrett at oracle.com
Thu Sep 19 17:27:20 UTC 2019


> On Sep 19, 2019, at 5:42 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
> 
> Hi Kim,
> 
>  thanks for your review.
> 
> On 18.09.19 20:36, Kim Barrett wrote:
>>> On Sep 18, 2019, at 11:22 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
>>> 
>>> Hi all,
>>> 
>>>  can I have reviews for this change that, given the observation that during root processing, there is only one kind of root that walks CLDs, we do not need to
>>> 
>>> [...]
>>> 
>>> (based on JDK-8231117)
>>> 
>>> Thanks,
>>>  Thomas
>> I was just looking at the root scanning code recently, trying to
>> figure out what worker_has_discovered_all_strong_classes() was about.
>> The naming of worker_has_discovered_all_strong_classes() and
>> wait_until_all_strong_classes_discovered() seems wrong now.  These are
>> about discovering/processing strong nmethods now.
>> A better comment at the call to the wait function about why it's
>> called there would be helpful.  Obviously we want it as late as
>> possible, to allow threads to get past the notification.  But why not
>> later?  
> 
> We could at the latest do the waiting when we do the nmethod weak root scanning. Given that I think I now have a working prototype that works without the barrier I did not care optimizing that for this change.
> If there are any unforeseen issues, or it does not pan out, I can improve that (in a separate CR if needed).
> 
> I fixed the naming of the two methods and tried to improve the comment in a new webrev.
> 
> New webrevs:
> http://cr.openjdk.java.net/~tschatzl/8159984/webrev.0_to_1/ (diff)
> http://cr.openjdk.java.net/~tschatzl/8159984/webrev.1/ (full)

The additional comments look good.  But shouldn’t the names of those functions refer to “nmethods"
rather than (generic) “roots”?  That is,

worker_has_discovered_all_strong_nmethods
wait_until_all_strong_nmethods_discovered

There are certainly strong roots that get looked at after the has_discovered call.

> 
>> (We talked about this off-line; this is just a reminder and
>> request for some commentary capturing that discussion. Though
>> hopefully your idea for getting rid of the barrier completly will pan
>> out.
>> Other than that, looks good.
> 
> Thanks!
> 
> Thomas





More information about the hotspot-gc-dev mailing list