[14] RFR(S): 8225670: compiler/types/correctness/* tests fail with "assert(recv == __null || recv->is_klass()) failed: wrong type"

[Patrick Zhang OS]

Hi Christian,

My test system shows this fix probably introduced a significant performance loss with a couple of apps/benchmarks on both aarch64 (Ampere eMAG) and x86 systems, I filed this ticket to track it. https://bugs.openjdk.java.net/browse/JDK-8230036

Regards
Patrick

-----Original Message-----
From: hotspot-compiler-dev <hotspot-compiler-dev-bounces at openjdk.java.net> On Behalf Of Christian Hagedorn
Sent: Friday, August 9, 2019 7:56 PM
To: Erik Österlund <erik.osterlund at oracle.com>; hotspot-compiler-dev at openjdk.java.net; Tobias Hartmann <tobias.hartmann at oracle.com>
Subject: Re: [14] RFR(S): 8225670: compiler/types/correctness/* tests fail with "assert(recv == __null || recv->is_klass()) failed: wrong type"

Hi Erik, hi Tobias

Thank you for your reviews!

On 09.08.19 11:21, Erik Österlund wrote:
> Hi Christian,
> 
> Looks good - well spotted.
> 
> To answer your question - yes the GC (ZGC in particular, and probably 
> soon Shenandoah when they hook in to the concurrent class unloading
> framework) cleans the extra data section of MDOs concurrently, under 
> the extra data lock.
> 
> However, they clear whole rows from the extra data section, under the 
> extra data lock of the MDOs; they never write that the Klass is NULL. 
> So I believe this bug only relates to the use of the WhiteBox API.

That was my guess, too. Thanks for clearing that up and answering the question.

> The row clearing of concurrent GCs synchronizes with a metadata 
> preparation phase for unpacking MDOs to ciMDOs. The preparation phase 
> will in a fixed-point iteration try to create ci handles for all 
> encountered metadata under the extradata lock. Every time it 
> encounters an uncached metadata instance, it has to release the lock 
> due to ranking issues, and may also run into safepoints then. Such 
> situations are detected, triggering a restart of the fixed-pont iteration.
> 
> Once the fixed-point iteration has finished, we know that we under the 
> lock walked all metadata in the extra data section without ever 
> releasing the lock, have ci handles keeping all metadata alive, and 
> can't have gotten any safepoints due to being in VM state. After that, 
> the rows are copied and translated, and now we are guaranteed that the 
> translation will always already have the ci handles cached.

Thanks for the detailed explanation!

> There is some random original copy of the raw MDO extra data that is 
> performed before preparing the metadata. I don't think it is really 
> used or needed. Might be interesting to remove in a future RFE. It 
> gets overwritten by the subsequent row-by-row processing after 
> metadata preparation.

I created a new RFE [1] and referenced this conversation.

Best regards,
Christian


[1] https://bugs.openjdk.java.net/browse/JDK-8229353