RFR(S): 8014408: G1: crashes with assert assert(prev_committed_card_num == _committed_max_card_num) failed

[John Cuthbertson]

Hi Thomas,

Thanks for the review.

Regarding your observations about the card table - do you want to submit 
a new CR for that, or shall I?

JohnC

On 5/15/2013 4:17 AM, Thomas Schatzl wrote:
> Hi,
>
> On Tue, 2013-05-14 at 14:15 -0700, John Cuthbertson wrote:
>> Hi Everyone,
>>
>> Can I have a couple of reviews of the fix for this assertion failure?
>> The webrev can be found at:
>> http://cr.openjdk.java.net/~johnc/8014408/webrev.0/
>>
>> Summary:
>> This was an assertion I recently added and is used as a consistency
>> check for the size of the card counts table. The problem was that there
>> was a mismatch between the setting of _committed_card_num and
>> prev_committed_card_num. An upper bound was used when setting the former
>> and this was missing when calculating the latter. It only showed up on a
>> system where the allocation alignment was 64k. I moved the calculation
>> of the number of cards for a given committed table size into a helper
>> routine and use the helper routine to set these values.
> The patch looks good; but there may be another issue here regarding heap
> sizing in G1:
>
> Afaik in hotspot the heap sizes must be aligned to multiples of what a
> single card table page spans.
> E.g. in this case, with 64k pages, this should be 32M (i.e. card_size *
> os::vm_page_size(), where card_size = 512 bytes).
>
> Since the card count table uses the same granularity (512 bytes), the
> card count table should have the same maximum size.
>
> This is the first odd thing: according to the comment, the MaxHeapSize
> is 1056350208, which is not evenly divisible by 32M.
> (In G1CollectedHeap::initialize(), line 2025 the code only enforces
> HeapRegion::GrainBytes as alignment for reservation - but in
> G1CollectorPolicy::initialize_flags() we set the max alignment to
> MAX2(HeapRegion::grain_bytes, card_table_alignment)...)
>
> All collectors (but G1) use this maximum alignment to align the heap at
> startup (they do, I checked); they also check that during resizing. But
> looking at g1CollectedHeap::expand, line 1779 and
> G1CollectedHeap::shrink_helper(), line 1848, G1 does not.
>
> (Looking at the log, the region size is 1M.)
>
> I think if there were a similar check in the card table code, it would
> fail probably fail the same - it's just that this tripped this
> inconsistency.
>
> I am not sure whether this behavior (that the card and card count tables
> can represent memory beyond the heap) is intended for G1. It seems that
> this is benign though, not sure.
>
> [With 4k pages we have a 2M minimum heap size granularity, so the issue
> may occur there as well...]
>
>> Testing:
>> The failing test cases on the system they fail on; spec jvm98 with MTT=0
> Thomas
>
>