RFR: 8287385: Suppress superficial unstable_if traps

[Vladimir Kozlov]

On Thu, 21 Jul 2022 19:54:11 GMT, Xin Liu <xliu at openjdk.org> wrote:

> An unstable if trap is **superficial** if it can NOT prune any code. Sometimes, the else-section of program is empty. The superficial unstable_if traps not only complicate code shape but also consume codecache. C2 has to generate debuginfo for them. If the condition changed, HotSpot has to destroy the established nmethod and compile it again.  Our analysis shows that rough 20% unstable_if traps are superficial. 
> 
> The algorithm which can identify and suppress superficial unstable if traps derives from its definition.  A non-superficial unstable_if trap must prune some code. Parser skips parsing dead basic blocks(BBs). A trap is superficial if and only if its target BB is not dead! Or, it will be skipped(contradict from definition). As a result, we can suppress an unstable_if trap when c2 parse the target BB. This algorithm leaves alone those uncommon_traps do prune code. 
> 
> For example, C2  generates an uncommon_trap for the else if cond is very likely true. 
> 
>     public static int foo(boolean cond, int i) {
>         Value x = new Value(0);
>         Value y = new Value(1);
>         Value z = new Value(i);
> 
>         if (cond) {
>             i++;
>         }
>         return x._value + y._value + z._value + i;
>     }
> 
> 
> If we suppress this superficial unstable_if, the nmethod reduces from 608 bytes to 520 bytes, or -14.5%. Most of them come from "scopes data/pcs". It's because superficial unstable_if generates a trap like this
> 
> 037     call,static  wrapper for: uncommon_trap(reason='unstable_if' action='reinterpret' debug_id='0')
>         # SuperficialIfTrap::foo @ bci:29 (line 32) L[0]=_ L[1]=rsp + #4 L[2]=#ScObj0 L[3]=#ScObj1 L[4]=#ScObj2 STK[0]=rsp + #0
>         # ScObj0 SuperficialIfTrap$Value={ [_value :0]=#0 }
>         # ScObj1 SuperficialIfTrap$Value={ [_value :0]=#1 }
>         # ScObj2 SuperficialIfTrap$Value={ [_value :0]=rsp + #4 }
>         # OopMap {off=60/0x3c}
> 03c     stop    # ShouldNotReachHere
> 
> 
> Here is the breakdown of nmethod, generated by '-XX:+PrintAssembly'
> 
> <-XX:-OptimizeUnstableIf>
> Compiled method (c2)     346   17       4       SuperficialIfTrap::foo (53 bytes)
>  total in heap  [0x00007f50f4970910,0x00007f50f4970b70] = 608
>  relocation     [0x00007f50f4970a70,0x00007f50f4970a80] = 16
>  main code      [0x00007f50f4970a80,0x00007f50f4970ad8] = 88
>  stub code      [0x00007f50f4970ad8,0x00007f50f4970af0] = 24
>  oops           [0x00007f50f4970af0,0x00007f50f4970b00] = 16
>  metadata       [0x00007f50f4970b00,0x00007f50f4970b08] = 8
>  scopes data    [0x00007f50f4970b08,0x00007f50f4970b38] = 48
>  scopes pcs     [0x00007f50f4970b38,0x00007f50f4970b68] = 48
>  dependencies   [0x00007f50f4970b68,0x00007f50f4970b70] = 8
> 
> <-XX:+OptimizeUnstableIf>
> Compiled method (c2)     309   17       4       SuperficialIfTrap::foo (53 bytes)
>  total in heap  [0x00007f4090970910,0x00007f4090970b18] = 520
>  relocation     [0x00007f4090970a70,0x00007f4090970a80] = 16
>  main code      [0x00007f4090970a80,0x00007f4090970ac8] = 72
>  stub code      [0x00007f4090970ac8,0x00007f4090970ae0] = 24
>  oops           [0x00007f4090970ae0,0x00007f4090970ae8] = 8
>  scopes data    [0x00007f4090970ae8,0x00007f4090970af0] = 8
>  scopes pcs     [0x00007f4090970af0,0x00007f4090970b10] = 32
>  dependencies   [0x00007f4090970b10,0x00007f4090970b18] = 8

Did you address @merykitty comment in RFE? You said: 
`it looks like this JBS does have this downsize, I will investigate this problem`

src/hotspot/share/opto/parse.hpp line 647:

> 645:   }
> 646: 
> 647:   void suppress(Parse* parser, Parse::Block* path);

Add comment describing this method.

src/hotspot/share/opto/parse1.cpp line 665:

> 663:           record_for_igvn(unc);
> 664:           //tty->print("mark dead: ");
> 665:           //unc->dump();

Debug lines left over?

src/hotspot/share/opto/parse2.cpp line 1586:

> 1584:   if (path_is_suitable_for_uncommon_trap(prob) && (!OptimizeUnstableIf || !path->is_merged())) {
> 1585:     sync_jvms();
> 1586:     SafePointNode* sfpt = clone_map();

Seems `sync_jvms()` and `clone_map() are only needed for `OptimizeUnstableIf`.
You need comment explaining why you added `!path->is_merged()` for `OptimizeUnstableIf` case.

-------------

PR: https://git.openjdk.org/jdk/pull/9601