RFR: JDK-8076542: G1 does not print heap page size information with -XX:+TracePageSizes
thomas.schatzl at oracle.com
Thu May 7 13:53:36 UTC 2015
On Thu, 2015-05-07 at 15:15 +0200, Bengt Rutisson wrote:
> Hi David,
> On 06/05/15 14:11, David Lindholm wrote:
> > Hi,
> > Please review this patch that adds a call to os::trace_page_sizes()
> > for G1, which prints heap page size info if run with -XX:+TracePageSizes
> > Webrev: http://cr.openjdk.java.net/~david/JDK-8076542/webrev.00/
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8076542
> 1864 ReservedSpace heap_rs = Universe::reserve_heap(max_byte_size,
> 1865 heap_alignment);
> 1867 size_t page_sz = os::page_size_for_region_aligned(max_byte_size, 1);
> 1868 os::trace_page_sizes("g1 heap",
> 1869 max_byte_size, page_sz,
> 1870 heap_rs.base(),
> 1871 heap_rs.size());
> I think it looks odd that we look up the page size that we print instead
> of actually getting the page size that was used.
Thinking about "TracePageSizes" and what is printed, it seems that it
confuses page size used for commit and reservation alignment.
> What I mean is that if we change the implementation of
> Universe::reserve_heap() to do something differently than using
> os::page_size_for_region_aligned() then this logging will suddenly be
> broken. I would prefer if we could somehow communicate what page size
> that was actually used. Or is it even the case that it is
> Universe::reserve_heap() that should do the call to os::trace_page_sizes()?
As mentioned above, it is strange to me to talk about page size when
reserving a memory area. I am not sure if we can or should fix the
output of TracePageSizes.
Actual page size used for committing is determined in line 1883 btw:
G1RegionToSpaceMapper* heap_storage =
os::large_page_size() : os::vm_page_size(), // <!---
Which means that the value actually used is different to the value
calculated in this change; it should be the same in all cases though,
but it would be nice to factor out this calculation and use it there and
when calling os::trace_page_sizes.
Also the kind string currently does not match the others printed
(created in create_aux_mapper()).
> Another thing, which is not really related to your change. There are two
> version of os::trace_page_sizes(). I couldn't find any user of this version:
> os::trace_page_sizes(const char* str, const size_t* page_sizes, int count)
> Can that one be removed?
It is used to print out all available page sizes on Solaris.
More information about the hotspot-gc-dev