RFR(S): 8214352: C1: Unnecessary "compilation bailout: block join failed" with JVMTI (off-list)

Thu Dec 13 16:16:41 UTC 2018

Ok, I think this makes sense. Looks good.

igor

> On Dec 13, 2018, at 2:59 AM, Doerr, Martin <martin.doerr at sap.com> wrote:
> 
> Hi Igor,
>  
> I understand that there are more important things to work on at the moment. Thanks for taking some time for reviewing my proposal.
>  
> With JVMTI “can_access_local_variables”, should_retain_local_variables() is true which keeps locals alive (ciMethod::liveness_at_bci) until they get reused (or a conflicting type).
> (For debugging, locals need to get kept alive within their scopes, but scope information may not be available, so C1 needs to be conservative enough.)
> So “can_access_local_variables” causes the “for_each_local_value” iterator to find locals which would otherwise have been considered dead out after their last usage.
>  
> In my schematic example, C1 calls setup_phi_for_local for the object in line 803 in recent c1_Instruction.cpp when coming from Block 1. When coming from Block 2, C1 reaches line 832 because “existing_state->is_same(new_state)”. The code after line 847 is not used.
>  
> Best regards,
> Martin
>  
> From: Igor Veresov <igor.veresov at oracle.com> 
> Sent: Donnerstag, 13. Dezember 2018 00:03
> To: Doerr, Martin <martin.doerr at sap.com>
> Cc: hotspot-compiler-dev at openjdk.java.net
> Subject: Re: RFR(S): 8214352: C1: Unnecessary "compilation bailout: block join failed" with JVMTI (off-list)
>  
> Martin,
>  
> Sorry I’ve got distracted from this review. I’m trying to understand why the phi is created in the first place. There is a code just below your change in c1_Instruction.cpp that should invalidate the local. Have you found out why doesn’t it work?
>  
> igor
> 
> 
> 
> 
> 
> On Dec 10, 2018, at 5:24 AM, Doerr, Martin <martin.doerr at sap.com <mailto:martin.doerr at sap.com>> wrote:
>  
> Hi Igor,
>  
> just let me know if you need more explanations. I know, the change is small, but not trivial.
> I can target this enhancement to jdk13 if you don’t have time at the moment even though it is tested very well.
>  
> The real life bailouts seem to be hard to analyze, but they can be observed often:
> jdk/bin/java -XX:+PrintBailouts -agentlib:jdwp=transport=dt_socket,address=8000,server=y,suspend=n -Djava.io.tmpdir=results -jar OctaneLauncher.jar
>  
> starting runNashorn.js
> compilation bailout: block join failed
> compilation bailout: block join failed
> compilation bailout: block join failed
> Richards: 95.9
> DeltaBlue: 71.9
> compilation bailout: block join failed
> compilation bailout: block join failed
> compilation bailout: block join failed
> Crypto: 315
> compilation bailout: block join failed
> RayTrace: 320
> EarleyBoyer: 652
> compilation bailout: too many stack slots used
> RegExp: 268
> compilation bailout: too many stack slots used
> Splay: 356
> SplayLatency: 228
> compilation bailout: block join failed
> compilation bailout: block join failed
> compilation bailout: block join failed
> compilation bailout: block join failed
> NavierStokes: 450
> compilation bailout: CodeBuffer overflow
> compilation bailout: CodeBuffer overflow
> compilation bailout: block join failed
> compilation bailout: block join failed
> compilation bailout: block join failed
> compilation bailout: block join failed
> compilation bailout: block join failed
> compilation bailout: CodeBuffer overflow
> compilation bailout: block join failed
> compilation bailout: block join failed
> compilation bailout: block join failed
> compilation bailout: block join failed
> … (also many other bailout reasons)
>  
> The “compilation bailout: block join failed” completely disappear with my change, so I still think this change is worth the effort.
>  
> Best regards,
> Martin
>  
>  
> From: hotspot-compiler-dev <hotspot-compiler-dev-bounces at openjdk.java.net <mailto:hotspot-compiler-dev-bounces at openjdk.java.net>> On Behalf Of Doerr, Martin
> Sent: Dienstag, 4. Dezember 2018 18:46
> To: Igor Veresov <igor.veresov at oracle.com <mailto:igor.veresov at oracle.com>>
> Cc: Vladimir Kozlov <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>>; hotspot-compiler-dev at openjdk.java.net <mailto:hotspot-compiler-dev at openjdk.java.net>
> Subject: [CAUTION] RE: RFR(S): 8214352: C1: Unnecessary "compilation bailout: block join failed" with JVMTI
>  
> Hi Igor,
>  
> thank you for looking at my proposal.
> Let me try to explain it by a schematic example.
>  
> Block 1
> local 1 used for some object (assignment and usage)
>  
> Block 2
> local 1 used for an int (assignment and usage)
> loop: if ... goto Block 2
>  
> C1 creates a Phi function (with input local 1 of type object) when looking at the edge from Block 1 to Block 2 (GraphBuilder::iterate_bytecodes_for_block => BlockBegin::try_merge => setup_phi_for_local).
> When looking at the backedge (the loop in Block 2), BlockBegin::try_merge sees the "existing_value" of type object and the "new_value" of type int coming from the backedge.
> This is the point at which we know that we are beyond the life ranges of local l. It's dead because a life local can't have conflicting types.
>  
> I think the appropriate treatment is to invalidate the local and the Phi function. Seems like an old version had already done this according to the comment "The old code invalidated the phi function here" probably because linear scan had issues and the problematic case was rare enough (without JVMTI). But now we have a bailout in LIRGenerator::move_to_phi which handles the really problematic case, so we can use the invalidation in BlockBegin::try_merge again which is far better when using JVMTI.
>  
> AFAIK, this matches what C2 would do.
>  
> Hope this makes it more clear. Let me know if I missed anything.
>  
> Thanks and best regards,
> Martin
>  
>  
> From: Igor Veresov <igor.veresov at oracle.com <mailto:igor.veresov at oracle.com>> 
> Sent: Dienstag, 4. Dezember 2018 18:13
> To: Doerr, Martin <martin.doerr at sap.com <mailto:martin.doerr at sap.com>>
> Cc: Vladimir Kozlov <vladimir.kozlov at oracle.com <mailto:vladimir.kozlov at oracle.com>>; hotspot-compiler-dev at openjdk.java.net <mailto:hotspot-compiler-dev at openjdk.java.net>
> Subject: Re: RFR(S): 8214352: C1: Unnecessary "compilation bailout: block join failed" with JVMTI
>  
> So how and why does it happen that a phi has different input types?
>  
> igor
> 
> 
> 
> 
>  
> 
> On Dec 3, 2018, at 2:24 AM, Doerr, Martin <martin.doerr at sap.com <mailto:martin.doerr at sap.com>> wrote:
>  
> Hi Vladimir,
> 
> thanks for reviewing. I hope a C1 expert can find some time to take a look.
> 
> Best regards,
> Martin
> 
> 
> -----Original Message-----
> From: hotspot-compiler-dev <hotspot-compiler-dev-bounces at openjdk.java.net <mailto:hotspot-compiler-dev-bounces at openjdk.java.net>> On Behalf Of Vladimir Kozlov
> Sent: Sonntag, 2. Dezember 2018 01:50
> To: hotspot-compiler-dev at openjdk.java.net <mailto:hotspot-compiler-dev at openjdk.java.net>
> Subject: Re: RFR(S): 8214352: C1: Unnecessary "compilation bailout: block join failed" with JVMTI
> 
> Seems fine to me but I am not C1 expert.
> 
> Regards,
> Vladimir
> 
> On 11/27/18 3:03 AM, Doerr, Martin wrote:
> 
> Hi,
> 
> C1 currently bails out when a local value is derived from a Phi with different types.
> 
> According to comments, this case is supposed to be very rare, but it's not so rare when a JVMTI agent is used 
> (capability "can_access_local_variables").
> 
> I'd like to fix this.
> 
> Issue with more detailed description:
> 
> https://bugs.openjdk.java.net/browse/JDK-8214352 <https://bugs.openjdk.java.net/browse/JDK-8214352>
> 
> Webrev:
> 
> http://cr.openjdk.java.net/~mdoerr/8214352_c1_phi/webrev.00/ <http://cr.openjdk.java.net/~mdoerr/8214352_c1_phi/webrev.00/> 
> <http://cr.openjdk.java.net/%7Emdoerr/8214352_c1_phi/webrev.00/ <http://cr.openjdk.java.net/%7Emdoerr/8214352_c1_phi/webrev.00/>>
> 
> Patch successfully ran through a substantial amount of tests with enabled "can_access_local_variables". Please review.
> 
> Best regards,
> 
> Martin
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/attachments/20181213/a1ee2d6d/attachment-0001.html>