RFR: 8211447: Replace oop_pc_update_pointers with oop_iterate and closure

Stefan Johansson stefan.johansson at oracle.com
Tue Oct 9 14:55:21 UTC 2018


Looks good,
StefanJ

On 2018-10-09 16:26, Leo Korinth wrote:
> Hi!
> 
> Here are new incremental and full webrevs that tries to improve the code 
> according to suggestions from Stefan and Thomas.
> 
> I did *not* change the name to pcClosure.inline.hpp as I think that was 
> a mistake (see my last e-mail).
> 
> incremental:
> http://cr.openjdk.java.net/~lkorinth/8211447/00-01/
> full:
> http://cr.openjdk.java.net/~lkorinth/8211447/01/
> 
> Thanks,
> Leo
> 
> On 09/10/2018 14:12, Leo Korinth wrote:
>> Hi, and thanks for reviewing!
>>
>> On 09/10/2018 11:06, Thomas Schatzl wrote:
>>> Hi,
>>>
>>> On Thu, 2018-10-04 at 18:31 +0200, Leo Korinth wrote:
>>>> Hi!
>>>>
>>>> This is the is the third enhancement in a series of three to remove
>>>> Parallel GC specific behaviour from shared code. I will post all
>>>> three enhancements to the mailing list.
>>>>
>>>> For a description on what is changed see my earlier "RFR: 8201436:
>>>> Replace oop_ps_push_contents with oop_iterate and closure"
>>>>
>>>> Enhancement:
>>>> https://bugs.openjdk.java.net/browse/JDK-8211447
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~lkorinth/8211447/00/
>>>
>>> - psClosure.inline.hpp:103: superfluous "protected" keyword (probably
>>> pre-existing)
>>>
>>> - I *think* psClosure.inline.hpp should be called
>>> "pcClosure.inline.hpp" - i.e. please fix the already mentioned "ps" vs.
>>> "pc" naming issue. :)
>>
>> psClosure.inline.hpp is changed in the first patch for JDK-8201436 and 
>> not for this patch (actually extracted from psScavenge.inline.hpp to 
>> psClosure.inline.hpp without being changed to fix include logic, but 
>> that _is hard_ to see in my webrev). I have neither created nor 
>> renamed the closures, just moved them. Also, they seem to be used from 
>> the scavenge part if I read the code correctly, so I think they are 
>> correctly named to begin with. I guess you confused this change in my 
>> earlier patch (PS related) with this RFR right?
>>
>> I will try to incorporate all your other changes together with 
>> Stefan's improvements and create new webrevs!
>>
>> Thanks,
>> Leo
>>
>>>
>>> - maybe fix the spacing of the "public" keyword in
>>> instanceClassLoaderKlass.hpp:55 (pre-existing issue); actually it can
>>> be removed.
>>>
>>> Thanks,
>>>    Thomas
>>>
>>>



More information about the hotspot-gc-dev mailing list