RFR(M/L): 7176479: G1: JVM crashes on T5-8 system with 1.5 TB heap

[John Cuthbertson]

Hi Thomas,

On 4/10/2013 1:37 AM, Thomas Schatzl wrote:
> It's okay, it's only that your description in the email to the mailing
> list indicated otherwise to me.
> The comment in the code reads "// If we failed to allocate the counts
> table, return 0.". Maybe the comment could explicitly indicate that that
> means the cards will always be cold in that case.
>
> What does " 145   // If card_ptr is beyond the committed end of the
> counts table, return 0." actually mean if card_ptr is beyond the end of
> the count table? There was not enough memory?

That's it exactly. Basically we expand the counts table when we expand 
the heap. So there is a chance we could successfully commit additional 
space for the heap but fail to commit additional space for the counts 
table. Since this is not a critical table - we can live with it being 
smaller than it should be at the expense of losing the optimization for 
some cards.

>
> In resize() the assert at line 132 indicates that this is actually a
> failure, which seems somewhat inconsistent in the way this situation is
> handled in debug/production mode.

You mean this one:

     assert(_card_counts_storage.committed_size() == new_committed_size,
            "expansion commit failure");

I allow for a failed expansion. But if the expansion is successful I 
want to ensure that I got all that I asked for. That should be the case. 
If it turns out not to be the case then the assert is an indication that 
we have to revisit the code. In most places where we commit space, we 
assert that the entire commit was successful.

> One further nitpick: the general documentation for the new classes is
> within the class. It looks as if that documentation is part of the first
> member of the class.
>
> E.g.
>
> class XX {
>    // documentation for XX
> }
>
> instead of the seemingly more common
>
> // documentation for XX
> class XX {
> }
>
> Not sure what's the preference here.

Me neither but it looks like you are right. I'll move those comments.

Thanks again for looking over the code changes.

JohnC