RFR(xs): 8221517: G1: Reserved page size for heap can be wrong

[sangheon.kim at oracle.com]

Hi Thomas,

Thanks for the review.

On 3/29/19 3:46 AM, Thomas Schatzl wrote:
> Hi Sangheon,
>
> On Thu, 2019-03-28 at 20:47 -0700, sangheon.kim at oracle.com wrote:
>> Hi all,
>>
>> Can I have some reviews for the patch which fixes wrong G1 heap page
>> size issue?
>>
>> The page size which is used to decide G1 heap mapper comes from
>> preferred page size or heap alignment. However, if it came from heap
>> alignment, we have to limit to actual large page size if it is
>> larger than large page size.
> Thanks for catching this.
:)

>
>> I added a new jtreg test to check this problem. It relies on the
>> error message when printed if reserving memory with large page fails.
>> Unfortunately, Solaris has a bug that the error message is printed
>> even in success. To address this problem I filed 'JDK-8221526:
>> Solaris: large page reservation failure message is printed even in
>> success.' and the test is excluded on Solaris.
> - g1CollectedHeap.cpp:1578: s/alginment/alignment ,
> s/ResesrvedSpace/ReservedSpace
>
> - Could you rewrite line 1581 with:
>
> page_size = MIN2(rs.alignment(), os::large_page_size());
>
> That would be a lot shorter and as understandable than the three extra
> lines imho.
>
> - TestLargePageUseForHeap.java:28: maybe add "... is allocated using
> large pages of the appropriate size if available" to the @summary tag.
All fixed, thanks!

http://cr.openjdk.java.net/~sangheki/8221517/webrev.1
http://cr.openjdk.java.net/~sangheki/8221517/webrev.1_inc_0
Testing: local build

Thanks,
Sangheon


>
>> CR: https://bugs.openjdk.java.net/browse/JDK-8221517
>> webrev: http://cr.openjdk.java.net/~sangheki/8221517/webrev.0
>> Testing: hs-tier1 ~ 5
> Thanks,
>    Thomas
>
>