RFR: 8031686: G1: assert(_hrs.max_length() == _expansion_regions) failed

[Jon Masamitsu]

On 1/22/2014 8:41 AM, Stefan Johansson wrote:
> Hi,
>
> I've got some comments offline about the test and my initial 
> assumption that more than one GC would be tested the way the test was 
> written might have been wrong. I've now made the test a G1 only 
> regression test, to specifically test the issue reported in this bug.

Looks good.  What would you think about changing the
name of the test to TestG1TwoGbHeap?  Just so it's
obviously a G1 test.

Jon
>
> New webrev:
> http://cr.openjdk.java.net/~sjohanss/8031686/webrev.02/
>
> Thanks,
> Stefan
>
> On 2014-01-17 15:23, Stefan Johansson wrote:
>> Hi again,
>>
>> I've added a regression test for the issue. See the updated review at:
>> http://cr.openjdk.java.net/~sjohanss/8031686/webrev.01/
>>
>> The test is naive and just tries to run -version with a heap size of 
>> 2g. I don't explicitly set G1 as GC but make sure I will use the 
>> arguments given to jtreg, this will ensure testing more than one GC 
>> and also that we do not run with G1 on unsupported platforms.
>>
>> Please have a look at the test and let me know if you have any 
>> questions or suggestions.
>>
>> Thanks,
>> Stefan
>>
>> On 2014-01-16 14:43, Stefan Johansson wrote:
>>> Hi,
>>>
>>> Please review this small fix for:
>>> https://bugs.openjdk.java.net/browse/JDK-8031686
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~sjohanss/8031686/webrev.00/
>>>
>>> Summary:
>>> The failing assertion is caused by an erroneous calculation done 
>>> when setting up the regions array in HeapRegionSeq. When using a 
>>> large heap a pointer calculation will overflow (since ptrdiff_t is 
>>> signed) and this will in turn lead to the regions array being 
>>> created longer that it should be. For non debug builds this will 
>>> lead to increased memory usage and jmap will report a faulty value 
>>> for the number of regions. The fix is to use the 
>>> pointer_delta-function to do the calculation.
>>>
>>> Note: The bug is currently targeted for 8 but is defer-requested and 
>>> should be moved to 9 shortly. I plan to fix it in 9 and backport it 
>>> to 8u20.
>>>
>>> Testing:
>>> * Manually verified that we are able to start 32-bit VMs with more 
>>> than 2g of heap using a debug build.
>>>
>>> Thanks,
>>> Stefan
>>
>