RFR: 8156032: Clean up parallel GC specific code from vm/gc/shared/preservedMarks.cpp

[Leonid Mesnik]

It seems that I need one more review for this fix. Could anyone looks on it?

Leonid

On 09.06.2016 16:05, Leonid Mesnik wrote:
> Thank you
>
> Leonid
>
> On 09.06.2016 15:50, Stefan Karlsson wrote:
>> Hi Leonid,
>>
>> On 2016-06-09 14:30, Leonid Mesnik wrote:
>>> Hi
>>>
>>> Thank you for review. Updated webrev is here:
>>> http://cr.openjdk.java.net/~lmesnik/8156032/webrev.01/ 
>>> <http://cr.openjdk.java.net/%7Elmesnik/8156032/webrev.01/>
>>
>> Looks good.
>>
>> Thanks,
>> StefanK
>>
>>>
>>> See comments inline:
>>>
>>> On 08.06.2016 17:21, Stefan Karlsson wrote:
>>>> Hi Leonid,
>>>>
>>>> Thanks for cleaning this up!
>>>>
>>>> See comments inlined:
>>>>
>>>> On 2016-06-07 15:35, Leonid Mesnik wrote:
>>>>> Hi
>>>>>
>>>>> Could you please review following patch which remove PS specific 
>>>>> code from vm/gc/shared.
>>>>> Webrev: http://cr.openjdk.java.net/~lmesnik/8156032/webrev.00/ 
>>>>> <http://cr.openjdk.java.net/%7Elmesnik/8156032/webrev.00/>
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8156032
>>>>
>>>> http://cr.openjdk.java.net/~lmesnik/8156032/webrev.00/src/share/vm/gc/parallel/psPromotionManager.cpp.udiff.html 
>>>>
>>>> http://cr.openjdk.java.net/~lmesnik/8156032/webrev.00/src/share/vm/gc/shared/preservedMarks.hpp.udiff.html 
>>>>
>>>>
>>>> +class PSPreservedMarksTaskExecutor : public 
>>>> RestorePreservedMarksTaskExecutor
>>>>
>>>> +class SharedPreservedMarksTaskExecutor : public 
>>>> RestorePreservedMarksTaskExecutor
>>>>
>>>>
>>>> Should the subclasses also mention the word Restore?
>>> Fixed.
>>>>
>>>> ------
>>>>
>>>> http://cr.openjdk.java.net/~lmesnik/8156032/webrev.00/src/share/vm/gc/shared/preservedMarks.hpp.udiff.html 
>>>>
>>>>
>>>> +public:
>>>> + WorkGang* workers;
>>>> +
>>>>
>>>>
>>>> workers should be private and named _workers.
>>> Fixed. Also PSRestorePreservedMarksTaskExecutor is updated to have 
>>> _gc_task_manager as a private member.
>>>>
>>>> ---
>>>>
>>>> + void restore(PreservedMarksSet* preserved_marks_set,
>>>> + volatile size_t* total_size_addr);
>>>>
>>>> Would you mind aligning the parameters?
>>> Done.
>>>>
>>>> ---
>>>>
>>>> + // Iterate over all stacks, restore all preserved marks, and reclaim
>>>> + // the memory taken up by the stack segments.
>>>> + // Supported executors: WorkerPresevedMarksTaskExecutor (Serial, 
>>>> CMS, G1),
>>>> + // PSManagerTaskExecutor (PS).
>>>>
>>>>
>>>> The comment refers to the old names of subclasses of 
>>>> RestorePreservedMarksTaskExecutor.
>>> Comments have been updated with current names.
>>>
>>> Leonid
>>>>
>>>> Thanks,
>>>> StefanK
>>>>
>>>>>
>>>>> Fix is tested with JPRT.
>>>>>
>>>>> Leonid
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20160609/69244d33/attachment.htm>