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

[Leonid Mesnik]

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/>

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