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