RFR: 8252221: Use multiple workers for Parallel GC pre-touching

[Thomas Schatzl]

Hi Amit,

   thanks for your contribution.

On 16.09.20 18:18, Amit Pawar wrote:
> Please review this change.


^^--- please consider putting a few lines of explanation in there like 
in a regular (old-style) RFR email. People may respond more eagerly if 
they do not have to search around what you want to accomplish.

Thanks.

> 
> -------------
> 
> Commit messages:
>   - 8252221: Improves JVM startup time for ParallelGC Pretouch using multiple threads
> 
> Changes: https://git.openjdk.java.net/jdk/pull/180/files
>   Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=180&range=00
>    Issue: https://bugs.openjdk.java.net/browse/JDK-8252221
>    Stats: 99 lines in 7 files changed: 82 ins; 4 del; 13 mod
>    Patch: https://git.openjdk.java.net/jdk/pull/180.diff
>    Fetch: git fetch https://git.openjdk.java.net/jdk pull/180/head:pull/180
> 
> PR: https://git.openjdk.java.net/jdk/pull/180
> 

- I am not sure why you want some callers of MutableSpace::initialize() 
not use the workgang for parallel pretouch. Particularly after resizing 
the old gen it might be very good to use it.

Just drop all optional parameters to intialize then, as I would prefer 
to have the Workgang as the last parameter.

- mutableSpace.cpp:166

       if (pretouch_gang) {

coding guidelines prohibit relying on implicit conversion of integers to 
booleans. Please add != NULL. See below too.

- mutableSpace.cpp:170/184:

the optimization to not call pretouching code just complicates the code: 
you do not save anything interesting as pretouching takes much longer 
than that check.

- I am not completely clear why the change only parallelizes the "head" 
portion of the MutableSpace in the initialize() method.

- afaict the current pretouch code does not pretouch the tail of the 
requested memory range if it is not evenly divisible by #threads and 
page size. Is this intentional?

- there are some other issues in the new gangtask code, but instead of 
rolling your own pretouch task in this change, please move the code in 
G1PageBasedVirtualSpace::pretouch() (changing parameters to use 
start_addr/end_addr and a page_size parameter in addition to the 
workgang) to a new class with that method as public static in the 
gc/shared directory and use it in both places.

Remove the "G1" prefixes in the shared code of course.

That pretouch gang task code one has been tested and if changes are 
required, they do not need to be done in multiple places.

Thanks,
   Thomas