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

Andrey Zakharov 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.

Thanks.

03.03.2015 18:40, Andrey Zakharov пишет:
> Fixed according comments.
>
> hotspot webrev:
> http://cr.openjdk.java.net/~azakharov/8061715/webrev.06/
>
> hs-gc webrev:
> http://cr.openjdk.java.net/~azakharov/8061715/webrev.06/hs-gc/
>
> testing: in progress
>
> Thanks.
>
> 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
>>>
>>> webrev:
>>> http://cr.openjdk.java.net/~azakharov/8061715/webrev.05/
>> - 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)
>>>
>>> Thanks.
>>>
>>>>>> - in TestShrinkAuxiliaryData.java, line 107 seems to be debugging 
>>>>>> code.
>>>>>> Afaik jtreg already automatically puts all output into the log 
>>>>>> files.
>>> 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.
>>> Fixed
>>>>>> - 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 ">"
>> then.
>>
>> 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
>> clear.
>>
>> Thanks,
>>    Thomas
>>
>>
>>
>




More information about the hotspot-gc-dev mailing list