RFR: 8027915: TestParallelHeapSizeFlags fails with unexpected heap size on sparcv9

[Stefan Johansson]

Hi Erik,

Thanks for making page_size_for_region more strict. A few comments:

src/share/vm/runtime/os.cpp

With this fix page_size_for_region doesn't care about alignment at all 
(old version wasn't much better) and to me that feels wrong. I would 
expect to get a page size that the given region sizes is aligned to, but 
I might be missing some use case where that isn't needed.

src/share/vm/gc_implementation/parallelScavenge/generationSizer.cpp

For the heap we want both min and max to be a multiple of the page size 
so we shoud call page_size_for_region for both _min_heap_byte_size and 
_max_heap_byte_size and use the smallest of the two returned page sizes.

src/share/vm/memory/heap.cpp

The current patch will not change the default behavior so I'm fine with 
just calling page_size_for_region once using reserved_size, but someone 
with more insight might know if we should handle it the same way as the 
generationSizer and use both committed and reserved.

Thanks,
Stefan


On 2014-05-06 10:44, Erik Helin wrote:
> Hi all,
>
> this patch changes
> os::page_size_for_region(min_size, max_size, min_pages) to always
> guarantee that the returned page size <= max_size / min_pages. Prior
> to this patch, page_size_for_region could return a page size larger
> than max_size / min_pages. This can cause some unexpected behavior
> for code using page_size_for_region, such as the following regression 
> tests:
>
> http://cr.openjdk.java.net/~ehelin/8027915/regression-test/
>
> As the test shows, if you have a 2 MB region and you must use at least
> 2 pages, you would still get back 2 MB as the page size on a Linux
> machine that supports 2 MB large pages. The reason for this is
> explained in a comment above the function in os.hpp:
>
> // The current implementation ignores min_pages if a larger page
> // size is an exact multiple of both region_min_size and
> // region_max_size.  This allows larger pages to be used when doing
> // so would not cause fragmentation; in particular, a single page can
> // be used when region_min_size == region_max_size == a supported page
> // size.
>
> Reducing fragmentation of large pages is good but the varying return 
> value makes it very hard to depend on os::page_size_for_region in 
> calculations (for example when sizing the heap).
>
> This patch removes the fragmentation prevention mechanism, simplifies
> the function and adds a regression tests:
>
> http://cr.openjdk.java.net/~ehelin/8027915/webrev.00/
>
> Testing:
> - JPRT
>
> Thanks,
> Erik