RFR: JDK-8076542: G1 does not print heap page size information with -XX:+TracePageSizes
david.lindholm at oracle.com
Fri May 8 08:09:10 UTC 2015
Thanks Thomas! A new webrev based on your feedback is available here:
We could have a discussion about what to print when TracePageSizes is
enabled (for all collectors), but I think it could be done outside this bug.
On 2015-05-07 15:53, Thomas Schatzl wrote:
> On Thu, 2015-05-07 at 15:15 +0200, Bengt Rutisson wrote:
>> Hi David,
>> On 06/05/15 14:11, David Lindholm wrote:
>>> 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 =
> UseLargePages ?
> 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