RFR: 8271195: Use largest available large page size smaller than LargePageSizeInBytes when available [v11]

[Thomas Schatzl]

On Mon, 28 Feb 2022 13:08:34 GMT, Swati Sharma <duke at openjdk.java.net> wrote:

>> Hi Team,
>> 
>> In this patch I have fixed two issues related to large pages, following is the summary of changes :-
>> 
>> 1. Patch fixes existing large page allocation functionality where if a commit over 1GB pages fails allocation should happen over next small page size i.e. 2M where as currently its happening over 4kb pages resulting into significant TLB miss penalty.
>> Patch includes new JTREG Test case covering various scenarios for checking the correct explicit page allocation according ​to the 1G, 2M, 4K priority.
>> 2. While attempting commit over larger pages we first try to reserve requested bytes over the virtual address space, in case commit to large page fails we should be un reserving entire reservation to avoid leaving any leaks in virtual address space.
>> 
>> ​
>> Please find below the performance data with and without patch for the JMH benchmark included with the patch.
>> 
>> ![image](https://user-images.githubusercontent.com/96874289/152189587-4822a4ca-f5e2-4621-b405-0da941485143.png)
>> 
>> 
>> Please review and provide your valuable comments.
>> 
>> 
>> 
>> Thanks,
>> Swati Sharma
>> Runtime Software Development Engineer 
>> Intel
>
> Swati Sharma has updated the pull request incrementally with one additional commit since the last revision:
> 
>   8271195: Resolved the comments.

Please drop the test.

The test is not able to distinguish failures from problems with the VM logic and other processes spuriously allocating enough large pages to make this test fail afaict. That's just a too annoying failure to debug. We can look into the test in a separate CR.

I kept the other comments I had for reference.

test/hotspot/jtreg/runtime/os/TestExplicitPageAllocation.java line 30:

> 28:  * @summary Use largest available large page size smaller than LargePageSizeInBytes when available.
> 29:  * @requires os.family == "linux"
> 30:  * @requires vm.gc != "Z"

Add a `requires vm.bits == 64` here - it is unlikely that any VM with 2g+ heap can be started on a 32 bit machine.

test/hotspot/jtreg/runtime/os/TestExplicitPageAllocation.java line 62:

> 60:     private static int[] pageCount = new int[MAX_NUMBER_OF_PAGESIZE];
> 61:     private static int largestPageSizeIndex;
> 62:     private static int vmPageSizeIndex;

As others suggested already, all the management and handling of configured page sizes using bit index is very complicated. Please use a simple array of sizes.
This also allows the use of existing query/sort method(s) if needed instead of rolling your own (i.e. the for-loops).

test/hotspot/jtreg/runtime/os/TestExplicitPageAllocation.java line 74:

> 72:            System.out.println("Exception " + e);
> 73:         }
> 74:     }

Just let the main method throw the exception - it is printed by default anyway.

test/hotspot/jtreg/runtime/os/TestExplicitPageAllocation.java line 76:

> 74:     }
> 75: 
> 76:     private static long requiredPageCount(int index) {

Add documentation of what this (and any other non-obvious) method actually calculates.

test/hotspot/jtreg/runtime/os/TestExplicitPageAllocation.java line 169:

> 167:                     // Only consider this a test failure if there are enough configured
> 168:                     // pages to allow this reservation to succeeded.
> 169:                     if (requiredPageCount(i) <= pageCount[i]) {

Aren't we going to get false positives if another test run concurrently allocates away all large pages?

test/hotspot/jtreg/runtime/os/TestExplicitPageAllocation.java line 178:

> 176:         }
> 177: 
> 178:         if (errorMessage != "") {

`errorMessage` seems to be never assigned to - this code seems to be dead. Remove.

-------------

Changes requested by tschatzl (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/7326