RFR (M): 8157952: Parallelize Memory Pretouch

sangheon sangheon.kim at oracle.com
Thu Sep 15 16:59:35 UTC 2016


Hi Thomas,

On 09/15/2016 01:24 AM, Thomas Schatzl wrote:
> Hi Sangheon,
>
> On Tue, 2016-09-13 at 16:16 -0700, sangheon wrote:
>> Hi Thomas,
>>
>> On 09/08/2016 04:47 AM, Thomas Schatzl wrote:
>>> Hi Jon,
>>>
>>>     a new webrev is available at http://cr.openjdk.java.net/~tschatz
>>> l/815
>>> 7952/webrev.1/ (full) and http://cr.openjdk.java.net/~tschatzl/8157
>>> 952/
>>> webrev.0_to_1/ (diff).
>> Looks good in general.
>>
>> http://cr.openjdk.java.net/~tschatzl/8157952/webrev.1/src/share/vm/ru
>> ntime/globals.hpp.frames.html
>> Please set a range or constraint for the new flag.
>> Probably range(1, SIZE_MAX / ParallelGCThreads) ?
>> I recommend to run
>> 'runtime/CommandLine/OptionsValidation/TestOptionsWithRanges.java'
>> for all platforms.
> Done. No issues.
Great!

>
>> Very minor comments, so you can ignore below.
>> http://cr.openjdk.java.net/~tschatzl/8157952/webrev.1/src/share/vm/gc
>> /g1/g1RegionToSpaceMapper.cpp.frames.html
>> line 70, 72
>> Can we set a new variable to avoid repeating '(size_t)start_idx *
>> _pages_per_region'?
>>
>> line 117
>> Probably 'size_t const NoPage'?
>>
>> http://cr.openjdk.java.net/~tschatzl/8157952/webrev.1/src/share/vm/gc
>> /g1/heapRegionManager.cpp.frames.html
>> http://cr.openjdk.java.net/~tschatzl/8157952/webrev.1/src/share/vm/gc
>> /g1/heapRegionManager.hpp.frames.html
>> Updating copyright year.
> All fixed.
>
> See
> http://cr.openjdk.java.net/~tschatzl/8157952/webrev.1_to_2 (diff)
> http://cr.openjdk.java.net/~tschatzl/8157952/webrev.2 (full)
Looks good.

Thanks,
Sangheon


>
> Thanks,
>    Thomas
>



More information about the hotspot-gc-dev mailing list