[13] RFR (S): 8188133: C2: Static field accesses in clinit can trigger deoptimizations

[Vladimir Ivanov]

> So this is the case for OSR compilation of <clinit> with hot loop 
> (iterations > 10000). Should we care about such cases?
> We want people to write simple <clinit> so we can serialize results - 
> this case is opposite direction.

It's definitely a corner case and having long-running static 
initializers is discouraged, but I don't agree that it should be ignored 
by JITs.

The faster <clinit> is finished the better for startup & warmup: as you 
noted, <clinit> serializes execution and may end up blocking the rest of 
the program. So, there are clear incentives to optimize <clinit>, though 
it may not be the most important case to care about.

> I understand that JITed code should perform better than Interpreter. And 
> we should fix regression in jdk 9.

Regression is handled separately (JDK-8192070) and it is not specific to 
static initializers. With this enhancement, I'm interested solely in how 
JITs handle <clinit>.

> There are several conditions for inlining. What if a called method is 
> not inlined even with your relaxed condition? We will still deoptimize 
> it on each iteration. Right?

Yes, proposed patch doesn't solve the general case. It just extends 
existing heuristics. If iniling fails, callee has to deoptimize.

> I am fine to allow access to static fields from <clinit> code but I am 
> not sure about allowing to inline general methods when class is not 
> fully initialized. Can we limit what methods should be inlined? Leaf 
> methods? Small code? No loops?

We definitely can, but I don't see compelling reasons to do so. It's 
perfectly fine to run any code in the context of static initializer. The 
only limitation is - only the thread which runs clinit is allowed to do 
so. I believe having clinit as the root of compilation is a strong 
guarantee of that invariant.

> I would prefer to not compile such <clinit> at all (if it has hot calls).

It would have negative consequences on startup and warmup.
In this particular case, the problem is not in clinit itself, but the 
code it calls. Methods accessing static members have the very same 
problem when compiled into a separate nmethod.

Best regards,
Vladimir Ivanov

> On 1/31/19 11:29 AM, Vladimir Ivanov wrote:
>> http://cr.openjdk.java.net/~vlivanov/8188133/webrev.00
>> https://bugs.openjdk.java.net/browse/JDK-8188133
>>
>> The test case in the bug demonstrates a pathological case with 
>> long-running static initializer: though it's allowed to access static 
>> fields from the thread performing the initialization, C2 can't prove 
>> that in general.
>>
>> While solving the general problem doesn't seem worth the effort 
>> (requires barriers on method entries), I propose to extend the logic 
>> to cover simple cases: when static initializer is the root of the 
>> compilation, all accesses to static fields of the corresponding class 
>> are allowed. It extends the coverage to all inlinees from OSR 
>> compilation of clinit.
>>
>> Testing: hs-precheckin-comp, tier1-5
>>
>> Best regards,
>> Vladimir Ivanov