[RFR] 8061715: gc/g1/TestShrinkAuxiliaryData15.java fails with java.lang.RuntimeException: heap decommit failed - after > before

[Thomas Schatzl]

Hi Andrey,

  sorry for the late review...

On Thu, 2015-02-12 at 17:00 +0300, Andrey Zakharov wrote:
> Hello.
> Summary: test for auxiliary data in G1 fails as they cannot precisely 
> measure this aux data size.
> I've added whitebox method to get this size.
> I've on little question about placement of helper method 
> "sum_memory_usage", which currently placed in
> gc_implementation/g1/heapRegionManager.hpp but it doesn't logically 
> related to HeapManager itself.
> If you can advice me where its best place it will be wonderful.
> 
> webrev:
> http://cr.openjdk.java.net/~azakharov/8061715/webrev/
> bug:
> JDK-8061715 gc/g1/TestShrinkAuxiliaryData15.java fails with 
> java.lang.RuntimeException: heap decommit failed - after > before
> 
> Testing done in Stockholm's JPRT (2015-02-12-112315.azakharov.hs-gc) all 
> is fine.

- the changes in the VM look good, there is one point though: the method
HeapRegionManager::get_auxiliary_data_memory_usage() not only returns
memory information about auxiliary data structures, but includes the
heap.
If this is intentional, the method name should be fixed to not read
"auxiliary".

- in the whitebox.cpp files, in WB_G1AuxiliaryMemoryUsage there seems to
be a superfluous newline at line 327/328.

- please fix copyright dates

- in TestShrinkAuxiliaryData.java, line 107 seems to be debugging code.
Afaik jtreg already automatically puts all output into the log files.

- line 174, I do not understand the comment. What does "if auxdata size
will be more than page size it would not decommit auxiliary data size is
about ~3.6% of heap size" seem to be at least two sentences.

- what do the two reasons given in the comment of
checkEnvApplicability() actually mean? I.e. why do you not run the test
if large pages are enabled?

I think the check is wrong too, it should check if UseLargePages is
enabled I think. You should use the HotSpotDiagnosticMXBean's
getVMOption() method.

Did you check that Whitebox.

The actual code does not help either.

The second check just seems to check whether
REGION_SIZE*REGIONS_TO_ALLOCATE is smaller than the heap size. Why not
specify -Xmx of 100M directly when starting the VM instead?

- it would be nice to not call the hot card cache "RSet cache". I do not
know why the switch has been called G1ConcRSLogCacheSize, but it does
not have to do much with the remembered set (it decreases the pressure
on the remembered set, but it has not a lot do to with it other than
that).

Thanks,
  Thomas