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

[Vladimir Kozlov]

I am more concern about C2 hidden assumptions about classes it compiles and static fields accesses order than about Java 
correctness of executing code in static initializers.

On other hand we are at the beginning of JDK 13 and can do this experiment to see how it goes. Okay but please test it 
may be up-to tier7 before push.

Changes itself seem fine.

thanks,
Vladimir K

On 2/1/19 10:13 AM, Vladimir Ivanov wrote:
> 
>> 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