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

[Swati Sharma]

On Fri, 4 Feb 2022 12:50:49 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: Review comments resolved.

> Hi @swati-sha and @tstuefe,
> 
> I was thinking the same thing, that you should see this PR :)
> 
> As mentioned this was discussed a lot in the review for [JDK-8256155](https://bugs.openjdk.java.net/browse/JDK-8256155). That said, I think there is room for improvement. What we settled on back then was that if any large page size passed the sanity check we consider large pages enabled. It can then of course look strange in some cases, but often when things are not working as expected it is because of some configuration "issue".
> 
> I suspect that the problem you are running into is that the default large page size is 1G, but you only have 2M pages configured. The simple work-around for this issue would be to specify `-XX:LargePageSizeInBytes=2m` but it would of course be good if it worked out of the box.
> 
> I did play around a bit trying to solve this, but it got shelved. I did try to solve it in the `ReservedSpace` layer instead. This would satisfy Thomas comment about not relying to much on checks at startup. I did something like:
> 
> ```diff
>     diff --git a/src/hotspot/share/memory/virtualspace.cpp b/src/hotspot/share/memory/virtualspace.cpp
> index c5f682bb31f..80d0910f150 100644
> --- a/src/hotspot/share/memory/virtualspace.cpp
> +++ b/src/hotspot/share/memory/virtualspace.cpp
> @@ -235,11 +235,20 @@ void ReservedSpace::reserve(size_t size,
>      // the caller requested large pages. To satisfy this request we use
>      // explicit large pages and these have to be committed up front to ensure
>      // no reservations are lost.
> +    size_t used_page_size = page_size;
> +    char* base = NULL;
> +
> +    do {
> +      base = reserve_memory_special(requested_address, size, alignment, used_page_size, executable);
> +      if (base != NULL) {
> +        break;
> +      }
> +      used_page_size = os::page_sizes().next_smaller(used_page_size);
> +    } while (used_page_size > (size_t) os::vm_page_size());
>  
> -    char* base = reserve_memory_special(requested_address, size, alignment, page_size, executable);
>      if (base != NULL) {
>        // Successful reservation using large pages.
> -      initialize_members(base, size, alignment, page_size, true, executable);
> +      initialize_members(base, size, alignment, used_page_size, true, executable);
>        return;
>      }
>      // Failed to reserve explicit large pages, fall back to normal reservation.
> ```
> 
> This is completely untested, and there might be some other tweaks needed to make it work. What do you think about this kind of approach?

Hi @kstefanj ,

Your proposed fix is not going to fix the alignment which will be incorrect wrt. the page_size and it may result 
into internal fragmentation within virtual address space which by the way is not a constraint but is a minor functional issue.
Also we are leaving incorrect value in _large_page_size(global variable) which is being used at several places.

Seek your opinion if above issues will not impact the functionality in any manner.

Thanks and Regards, 
Swati Sharma

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

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