RFR (XS): 8027756: assert(!hr->isHumongous()) failed: code root in humongous region? (P2-JDK8!)

[Tao Mao]

Hi Thomas,

Can you make the assertion message from a question to a statement? It'll 
help future readers understand it.

say, in g1CollectedHeap.cpp

- assert(!hr->continuesHumongous(), "code root in continuation of 
humongous region?");
+ assert(!hr->continuesHumongous(), "code root should not attached to a 
humonguous continuation");

There may be a couple of other similar ones.

Thanks.
Tao

On 11/5/13 5:46 AM, Thomas Schatzl wrote:
> Hi all,
>
>    can I get quick reviews for the following small changes?
>
> The patchset fixes wrong assertions that assume that references to
> humongous/large objects cannot be embedded into the code stream. They
> can.
>
> This is not true, and in the failing (existing) test case just that
> happens.
>
> The fix is to just change all these wrong assertions to check whether
> the region we want to add the code root to is a humongous continuation:
> that really should not occur as they do not contain an object header.
>
> Note that there is a similar assertions with the same conditions in e.g.
> MigrateCodeRootsHeapRegionClosure::doHeapRegion() - however this one
> checks a different situation: there should be no migration of code roots
> because (currently) large objects can never move.
>
> The original reasons for adding this assertion, and why this assertion
> only triggers just now (the code has been in for a few months) are
> unknown. I will try to find out what change (possibly in the compiler)
> triggered this.
>
> The changeset also adds a new test in addition to the original one that
> particularly targets most reasons why the assertion failed before:
>   - when adding a new code root to a humongous region
>   - when removing a new code root to a humongous region
>   - during verification (does anybody know why
> G1VerifyHeapRegionCodeRoots is false by default?)
>   - strong code roots root marking (in anticipation of class unloading
> during concurrent marking)
>
> bugs.openjdk
> https://bugs.openjdk.java.net/browse/JDK-8027756
>
> Webrev
> http://cr.openjdk.java.net/~tschatzl/8027756/webrev/
>
> Testing:
> jprt, manual invocation of the failing test (verifying that without the
> change the assertion triggers, and does not with the patch), new test
>
> Thanks,
>    Thomas
>