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

[Stefan Karlsson]

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/ef428864/attachment.htm>