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

[Jon Masamitsu]

Thomas,

Do you think there would be value in printing out the
region address in the cases where the assertion fails?

Does your new test case fail with both the client and
server compilesr?

Jon

On 11/5/2013 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
>