RFR(s): 8145000: TestOptionsWithRanges.java failure for XX:+UseNUMA -XX:+UseNUMAInterleaving -XX:NUMAInterleaveGranularity=65536

[sangheon]

Hi Jesper,

Thank you for looking at this!

On 12/16/2015 06:46 AM, Jesper Wilhelmsson wrote:
> Hi Sangheon,
>
> * It seems to me that protect_pages_individually() can only be called 
> if UseLargePages is false. Still it looks at UseLargePages to decide 
> page_size. I would prefer if there was an assert(!UseLargePages) at 
> the start of protect_pages_individually() to emphasize that this code 
> has not been used or tested with large pages.
>
> * I'm not sure if this matters, but it seems to me that if the size of 
> the area is not an even multiple of pages, you will start protecting 
> the smaller part, and then continue with page sized areas. E.g. If 
> page size is 4k and we want to protect a 10k area, we will start with 
> a 2k block and then take two 4k blocks. This has the effect that if we 
> pass in a page aligned address, the areas we protect will not cover 
> one page at a time. This may be a non-issue though. It depends on if 
> we usually get page aligned addresses and if it matters if we protect 
> one page at a time or not.
Your second comment made me to change the function which maybe doesn't 
need an assert now and  removed potential alignment problem.
Currently protect_pages_individually() is only called when 
"UseNUMAInterleaving && !UseLargePages" and protect_pages_indivisually() 
will always have aligned size as allocate_pages_individually() allocate 
aligned size. This means, as you pointed, we need an assert to prevent 
wrong use and would have alignment issue.

So I changed the function to read currently allocated chunk size(via 
Windows API) rather than calculating chunk size and then try to protect 
the chunk.
With this change, we don't need to care about the flags neither align 
matter.

>
> * You use UINT32_FORMAT to print an uint. %u is the recommended way to 
> print an uint.
Fixed.

>
> * if (!ret) return false;  We should always use { }
Fixed.

Here's a next webrev.
http://cr.openjdk.java.net/~sangheki/8145000/webrev.01/
http://cr.openjdk.java.net/~sangheki/8145000/webrev.01_to_00/

Testing: JPRT (TestOptionsWithRanges.java), RBT 
(hotspot_all,testlist,noncolo.testlist for Windows only)

* Added more tests.

Thanks,
Sangheon


>
> Thanks,
> /Jesper
>
>
>
> Den 16/12/15 kl. 01:53, skrev sangheon:
>> Hi all,
>>
>> Could I get a couple of reviews for Windows NUMAInterleaveGranularity 
>> change?
>>
>> Current Windows implementation can't handle correctly when we reserve 
>> a heap
>> with disjoint heap base mode with NUMAInterleaveGranularity.
>>
>> Windows, os::protect_memory fails in above case, as we are trying to 
>> protect the
>> whole reserved heap at one time. But we reserved that area with
>> separate/contiguous chunks based on NUMAInterleaveGranularity size. MSDN
>> describes to call the API separately.
>> I changed protect API to be called multiple times if needed.
>>
>> Skipped adding a test as TestOptionsWithRanges.java is enough.
>> 'java -XX:+UseNUMA -XX:+UseNUMAInterleaving' is enough to reproduce 
>> on large
>> memory Windows machine. However we need to specify heap size if the 
>> machine
>> doesn't have large memory. e.g. -Xms30g -Xmx30g
>>
>> This patch is based on the patch for JDK-8144949 which includes the 
>> change of
>> the max range of NUMAInterleaveGranularity to 2G(32bit), 8192G(64bit).
>>
>> CR: https://bugs.openjdk.java.net/browse/JDK-8145000
>> Webrev: http://cr.openjdk.java.net/~sangheki/8145000/webrev.00
>> Testing: JPRT (with TestOptionsWithRanges.java enabled), manual tests 
>> on Windows
>> machine(to test on large memory).
>>
>> Thanks,
>> Sangheon
>>
>>