[RFR] 8061715: gc/g1/TestShrinkAuxiliaryData15.java fails with java.lang.RuntimeException: heap decommit failed - after > before
andrey.x.zakharov at oracle.com
Tue Mar 3 16:59:49 UTC 2015
Testing as 2015-03-03-151040.gtee.auxdata looks good in Sth queue.
03.03.2015 18:40, Andrey Zakharov пишет:
> Fixed according comments.
> hotspot webrev:
> hs-gc webrev:
> testing: in progress
> 02.03.2015 17:57, Thomas Schatzl пишет:
>> Hi Andrey,
>> On Thu, 2015-02-26 at 20:55 +0300, Andrey Zakharov wrote:
>>> Updated test with
>>> - updated 2 spaces for c++ code and code style
>>> - updated copyrights
>>> - renamed variable hotCardTableSize
>>> - added output for current page size
>> - in HeapRegionManager::get_auxiliary_data_memory_usage(), please avoid
>> declaring and initializing using multiple variables in a single
>> statement, particularly if these statements span multiple lines.
>> While coding guidelines do not explicitly prohibit this, it is easier to
>> read if you repeat the type of the variable.
>> Also, it would be easier to read, if there were an additional line
>> between the return statement and the declaration of committed_sz. The
>> existing code also spells out "sz".
>> - the method bodies in the G1 code use an indentation of four spaces.
>> Hotspot code uses two space.
>> - not sure about this, but the changed tests do not add 8061715 to their
>> @bug tag. It will be impossible to test particularly 8061715 then
>> without finding the original bug number.
>> - the webrev is missing the changes to the WhiteBox.java file at
>> <top-level>/test/lib/sun/hotspot/WhiteBox.java. Please generate an extra
>> webrev for that repo.
>>> testing: yes (sthjprt 2015-02-26-172927.gtee.auxdata - I will restart
>>> solaris on complete)
>>>>>> - in TestShrinkAuxiliaryData.java, line 107 seems to be debugging
>>>>>> Afaik jtreg already automatically puts all output into the log
>>> This is why this code here: OutputAnalyzer with ProcessBuilder doesn't
>>> implicitly output anything.
>> Many other tests do not either (can be easily added for debugging), but
>> if so, I recommend putting the System.out.println() statement before the
>> output.shouldHaveExitValue(0); line. Otherwise the test will exit before
>> printing the messages if there is some VM error.
>>>>>> - 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 dont want to run this test on box where page size is larger than
>>> auxiliary data size for region, where decommitment wouldn't occurs
>>> for sure.
>> Okay. But then the comment is wrong: it states "if auxdata size will be
>> more than page size it would not decommit" which is the reverse of what
>> you said above.
>> I also think the comparison below should be a ">=" and not only ">"
>> The skip message could be improved to mention "Skipping test because the
>> auxiliary data is smaller than page size %d" to make the reason more
More information about the hotspot-gc-dev