RFR (M): 8157952: Parallelize Memory Pretouch

[Jon Masamitsu]

On 09/08/2016 03:26 AM, Thomas Schatzl wrote:
> Hi Jon,
>
> On Wed, 2016-09-07 at 14:37 -0700, Jon Masamitsu wrote:
>> Thomas,
>> http://cr.openjdk.java.net/~tschatzl/8157952/webrev/src/share/vm/gc/g
>> 1/g1_globals.hpp.frames.html
>> Could we change G1PreTouchChunkSize to PreTouchChunkSize?  It
>> seems like something we would want to do for other GC's at some
>> point.
> Can do. Maybe "ParallelPreTouchChunkSize"?

The "Parallel" in "ParallelPreTouchChunkSize" makes it more
restrictive in the sense that it implies the pretouch is going to
be done in parallel when, in the end, it might be done serially
(large value for ParallelPreTouchChunkSize, small value for
heap).  Also it seems longish to me.  I would just use PreTouchChunkSize.
Or maybe PreTouchSliceSize which for the really nerdy implies
parallel work :-).  Just my opinion.

>
>> http://cr.openjdk.java.net/~tschatzl/8157952/webrev/src/share/vm/gc/s
>> hared/workgroup.hpp.frames.html
>>
>>   64     _gc_id(Universe::is_fully_initialized() ? GCId::current_raw()
>> : 0)
>> What prompted the above change?
> The problem is that we need to use a work gang during initialization of
> the VM. GCId and the whole logging framework is not up at this point,
> and GCId::current_raw() returns -1 (invalid) that is checked in some
> asserts.
>
> That is also why the code only logs messages when we are not
> initializing the heap.
>
> I will see if there is another, more clean, way to enable the workgang
> framework during VM initialization.

I'd just like to avoid confusion if there is the possibility that a GCId 
of 0
is used twice.

>
>> http://cr.openjdk.java.net/~tschatzl/8157952/webrev/src/share/vm/gc/g
>> 1/g1PageBasedVirtualSpace.cpp.frames.html
>>
>> I don't think there is anything G1 specific about class
>> G1PretouchTask.  Could it be renamed PretouchTask and put into its
>> own file for easier reuse?  If you think that is premature, ignore
>> this comment.
> I do think that is premature.

Ok.

>
> Also, there will be need for a special version for parallel gc as it
> does not use the WorkGang framework... :(

:( It would require a little more work.  Leave as is for now.

>
> I am not sure we will need it for CMS at this time.

I'm also unsure so leave it out.

>
>> Pretouch() allows for the case where the work is done by the
>> current thread (only 1 chunk or null pretouch_gang).
>>    259 void G1PageBasedVirtualSpace::pretouch(size_t start_page,
>> size_t size_in_pages, WorkGang* pretouch_gang) {
>> Pretouching is so slow that I would not think it made a difference if
>> parallel worker were used instead of the current thread.  Simpler to
>> just always require a gang and use 1 worker in the degenerate case?
> Can do.

Thanks.

Jon
>
>> Rest looks good.
> I will provide an updated webrev later.
>
> Thanks,
>    Thomas
>