RFR: 8156032: Clean up parallel GC specific code from vm/gc/shared/preservedMarks.cpp
Leonid Mesnik
Leonid.Mesnik at oracle.com
Thu Jun 9 13:05:04 UTC 2016
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/a5ba73dc/attachment.htm>
More information about the hotspot-gc-dev
mailing list