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

[Thomas Schatzl]

Hi,

On 19.09.19 19:27, Kim Barrett wrote:
>> 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.

   fixed in place as suggested. The naming is indeed overloaded here; I 
wanted to make the names more generic than they need to be.

Thanks,
   Thomas