RFR: 8189359: Move native weak oops cleaning out of ReferenceProcessor

Stefan Karlsson stefan.karlsson at oracle.com
Tue Oct 17 21:22:54 UTC 2017


On 2017-10-17 23:11, Kim Barrett wrote:
>> On Oct 17, 2017, at 4:57 PM, Stefan Karlsson <stefan.karlsson at oracle.com> wrote:
>> Here are the updated webrevs:
>>   http://cr.openjdk.java.net/~stefank/8189359/webrev.01.delta
>>   http://cr.openjdk.java.net/~stefank/8189359/webrev.01

Obviously, this is getting too late for me to do these kinds of changes. 
But let me try one more time. :)

> ------------------------------------------------------------------------------
> src/hotspot/share/gc/shared/weakProcessor.hpp
>    33 // New contains of weak oops added to this class will automatically
>
> Sorry, but that's garbled, and I'm not sure what is intended.
>
> Previous version had "sets" instead of "contains", which seemed okay
> to me.

I used the word container in the comment for weak_oops_do, so I wanted 
to use the same word here. I can change to set(s) if that makes more sense.

> ------------------------------------------------------------------------------
> src/hotspot/share/gc/shared/weakProcessor.hpp
>    45   // Visit all oop*s and apply the given clousre.
>
> s/clousre/closure/

Done.

>
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/shared/weakProcessor.hpp
>    41   // The complete closure is used as a post-processing step called
>    42   // after each container has been processed.
>
> I think a comma is needed between "step" and "called".

Done.

>    But we were
> discussing in chat whether this closure is even needed.  I think it
> isn't...

I agree. But I'd rather think about that a bit more and then remove that 
as a separate patch.

>
> ------------------------------------------------------------------------------
> src/hotspot/share/gc/shared/weakProcessor.hpp
>    37   // Visit all oop*s and either apply the keep_alive closure if the referenced
>    38   // object is considered alive by the is_alive closure, otherwise do some
>    39   // container specific cleanup of element holding the oop.
>
> Suggest
>
> s/either//
> s/, otherwise/. Otherwise/
>
> ------------------------------------------------------------------------------
Done.

http://cr.openjdk.java.net/~stefank/8189359/webrev.02.delta
http://cr.openjdk.java.net/~stefank/8189359/webrev.02

Thanks,
StefanK

>



More information about the hotspot-dev mailing list