JDK-8286104 : use aggressive liveness for unstable_if traps
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 19
  • Priority: P4
  • Status: Open
  • Resolution: Unresolved
  • OS: generic
  • CPU: generic
  • Submitted: 2022-05-04
  • Updated: 2022-06-27
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
Other
tbdUnresolved
Related Reports
Relates :  
JDK-8276998 - [REDO] Optimization of Box nodes in uncommon_trap
Description
c2 parser is conservative about the bci unstable_if trap. It points to the conditional branch. When interpreter resumes execution,  it will reevaluate the condition branch and take another path.  unstable_trap collects liveness information at the bci and make live locals the input  parameters of CallStaticJava uncommon_trap node. 

We can aggressively set next bci for the unstable traps. Most of next BCIs (fall through or destination) are closer to the exit. It's likely to have less live locals.  There are 3 benefits. 

1. less inputs of uncommon_trap node can simplify IR. this may unblock other optimizations such as useless function elimination JDK-8276998 or scalar replacement. 

2. deoptimizations become faster because we don't need to reallocate those dead objects.

3. save codecache space. we store all debuginfo in codecache. for those scalarized objects, we have to save fields of them in debuginfo. we can avoid them if they are not live.
Comments
the problem of 'isDateBased()` has been solved by JDK-8287840.
20-06-2022
reproducible: $java -XX:-TieredCompilation -XX:CompileCommand=dontinline,java.time.LocalDate::isSupported -Xbatch -XX:CompileCommand=PrintOptoAssembly,java.time.LocalDate::isSupported -XX:+TraceDeoptimization DateTimeTest Problem Analysis One fold-compares is postponed to iterGVN2. it's because a IF node's control has a weird region node 194. it's a copy of 150(its parent). 150 IfFalse === 148 [[ 194 205 ]] #0 Type:control !jvms: java.time.temporal.ChronoField::isDateBased @ bci:10 (line 687) java.time.chrono.ChronoLocalDate::isSupported @ bci:8 (line 388) 194 Region === _ 150 [[]] Type:control !jvms: java.time.temporal.ChronoField::isDateBased @ bci:20 (line 687) java.time.chrono.ChronoLocalDate::isSupported @ bci:8 (line 388) 205 If === 150 204 [[ 206 207 ]] P=1.000000, C=6784.000000 Type:{0:control, 1:control} !jvms: java.time.temporal.ChronoField::isDateBased @ bci:23 (line 687) java.time.chrono.ChronoLocalDate::isSupported @ bci:8 (line 388)
04-06-2022
So far I see only these 2 failures during tier1-4 testing. Running later tiers now.
03-06-2022
Performance results are mixture of small improvements and regressions (could be due to variations between runs) but mostly neutral. I compared the same build with switching on/off AggressiveLivenessForUnstableIf flag.
03-06-2022
The expression at ChronoField.java:687 is likely a candidate of 'fold-compares'. public boolean isDateBased() { return ordinal() >= DAY_OF_WEEK.ordinal() && ordinal() <= ERA.ordinal(); } I will figure out how trigger this issue and debug it.
03-06-2022
2 tests failed in tier2: tools/jar/CreateMissingParentDirectories.java java/time/tck/java/time/TCKOffsetDateTime.java They have the similar issue when parsing date string ("this" is null): java.lang.ExceptionInInitializerError at jdk.jartool/sun.tools.jar.Main.parseArgs(Main.java:519) at jdk.jartool/sun.tools.jar.Main.run(Main.java:257) at jdk.jartool/sun.tools.jar.JarToolProvider.run(JarToolProvider.java:42) at java.base/java.util.spi.ToolProvider.run(ToolProvider.java:162) at CreateMissingParentDirectories.doHappyPathTest(CreateMissingParentDirectories.java:84) at CreateMissingParentDirectories.realMain(CreateMissingParentDirectories.java:66) at CreateMissingParentDirectories.main(CreateMissingParentDirectories.java:114) at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104) at java.base/java.lang.reflect.Method.invoke(Method.java:578) at com.sun.javatest.regtest.agent.MainActionHelper$AgentVMRunnable.run(MainActionHelper.java:312) at java.base/java.lang.Thread.run(Thread.java:1596) Caused by: java.time.format.DateTimeParseException: Text '2099-12-31T23:59:59Z' could not be parsed: Cannot invoke "java.time.temporal.ChronoField.ordinal()" because "this" is null at java.base/java.time.format.DateTimeFormatter.createError(DateTimeFormatter.java:2077) at java.base/java.time.format.DateTimeFormatter.parse(DateTimeFormatter.java:2012) at java.base/java.time.ZonedDateTime.parse(ZonedDateTime.java:600) at java.base/java.time.ZonedDateTime.parse(ZonedDateTime.java:585) at jdk.jartool/sun.tools.jar.GNUStyleOptions.<clinit>(GNUStyleOptions.java:50) ... 11 more Caused by: java.lang.NullPointerException: Cannot invoke "java.time.temporal.ChronoField.ordinal()" because "this" is null at java.base/java.time.temporal.ChronoField.isDateBased(ChronoField.java:687) at java.base/java.time.chrono.ChronoLocalDate.isSupported(ChronoLocalDate.java:388) at java.base/java.time.LocalDate.isSupported(LocalDate.java:533) at java.base/java.time.format.Parsed.crossCheck(Parsed.java:707) at java.base/java.time.format.Parsed.crossCheck(Parsed.java:693) at java.base/java.time.format.Parsed.resolve(Parsed.java:267) at java.base/java.time.format.DateTimeParseContext.toResolved(DateTimeParseContext.java:331) at java.base/java.time.format.DateTimeFormatter.parseResolved0(DateTimeFormatter.java:2112) at java.base/java.time.format.DateTimeFormatter.parse(DateTimeFormatter.java:2008)
03-06-2022
The reason I move this logic after iGVN because I think we have seen all IfNode after one pass of iGVN. A failure reported by Tobias reveals that there's a loophole. https://github.com/openjdk/jdk/pull/8545#issuecomment-1131316937 Some IfNodes postpone fold-compares after loop optimizations because range check may further simplify it. diff --git a/src/hotspot/share/opto/ifnode.cpp b/src/hotspot/share/opto/ifnode.cpp index 88d0843ce89..14ecc3522d8 100644 --- a/src/hotspot/share/opto/ifnode.cpp +++ b/src/hotspot/share/opto/ifnode.cpp @@ -1045,6 +1045,8 @@ bool IfNode::fold_compares_helper(ProjNode* proj, ProjNode* success, ProjNode* f igvn->remove_dead_node(adjusted_lim); } igvn->C->record_for_post_loop_opts_igvn(this); + // process_unstable_ifs precedes loop optimization. just bail out. + dom_iff->set_unc_bci(-1); return false; } }
20-05-2022
This idea is challenged by a structural optimization called fold-compares in IfNode. This transformation fuses 2 signed integer comparisons into one unsigned one. If both 2 IfNode has uncommon_traps, it will use the dominating one to cover 2 IfNodes. This actually change the semantics of unstable_if trap and ask him to cover 2 branches. It breaks my original assumption that unstable_if trap lead to the other path when execution resumes in interpreter. eg. j.l.Short.valueOf() has a cache. expression "sAsInt >= -128 && sAsInt <= 127" is the candidate of fold-compares. A unit test 'ShortTest.java' is uploaded. @IntrinsicCandidate public static Short valueOf(short s) { final int offset = 128; int sAsInt = s; if (sAsInt >= -128 && sAsInt <= 127) { // must cache return ShortCache.cache[sAsInt + offset]; } return new Short(s); } After fold-compares, the unstable_if trap in B3 needs to cover both 'sAsInt < -128' and 'sAsInt > 127' ------------------------ OptoAssembly for Compile_id = 26 ----------------------- # # java/lang/Short:exact * ( int ) # 000 N1: # out( B1 ) <- in( B3 B2 ) Freq: 1 000 B1: # out( B3 B2 ) <- BLOCK HEAD IS JUNK Freq: 1 000 # stack bang (144 bytes) pushq rbp # Save rbp subq rsp, #32 # Create frame 00c movl R11, RSI # spill 00f addl R11, #128 # int 016 cmpl R11, #256 # unsigned 01d jnb,us B3 P=0.000000 C=5375.000000 01f B2: # out( N1 ) <- in( B1 ) Freq: 1 01f movslq R10, RSI # i2l 022 movq R11, narrowoop: java/lang/Short:NotNull:exact *[int:256]<ciObjArray length=256 type=<ciObjArrayKlass name=[Ljava/lang/Short; ident=1271 address=0x00007f3904307610> ident=1274 address=0x00007f39043080c0> * # ptr 02c movl R10, [R11 + #528 + R10 << #2] # compressed ptr 034 decode_heap_oop_not_null RAX,R10 038 addq rsp, 32 # Destroy frame popq rbp cmpq rsp, poll_offset[r15_thread] ja #safepoint_stub # Safepoint: poll for GC 04a ret 04b B3: # out( N1 ) <- in( B1 ) Freq: 4.76837e-07 04b movl [rsp + #0], RSI # spill 04e movl [rsp + #4], RSI # spill 052 movl RSI, #-195 # int 057 call,static wrapper for: uncommon_trap(reason='unstable_fused_if' action='reinterpret' debug_id='0') # java.lang.Short::valueOf @ bci:9 (line 277) L[0]=rsp + #0 L[1]=_ L[2]=rsp + #4 STK[0]=rsp + #0 STK[1]=#-128 # OopMap {off=92/0x5c} 05c stop # ShouldNotReachHere To be honest, I never expect this before.It's hard to reconcile because fold-compares could happen everywhere. it's not safe to kill any local based on speculative bci until we can guarantee that fold-compares won't happen. that make implementation nasty.
20-05-2022
for Test::foo, run with the follow cmd. $java -XX:CompileCommand=compileonly,Test.foo -XX:CompileCommand=dontinline,Test.black -Xbatch -XX:+PrintEliminateAllocations -XX:+PrintEscapeAnalysis -XX:+TraceDeoptimization -XX:+PrintDeoptimizationDetails -XX:-AggressiveLivenessForUnstableIf Test the object 153 can't been scalarized because it's NSR, which is rendered by 94 PHI. the only use of that phi is unstable if trap. ======== Connection graph for Test::foo invocation #0: 2 iterations and 0.000010 sec to build connection graph with 240 nodes and worklist size 15 JavaObject(4) NoEscape(NoEscape) NSR [ 228F [ 165 170 94 ]] 153 Allocate === 117 6 7 8 1 ( 141 151 40 1 1 10 1 1 10 ) [[ 154 155 156 163 164 165 ]] rawptr:NotNull ( int:>=0, java/lang/Object:NotNull *, bool, top, bool ) Integer::valueOf @ bci:23 (line 1074) Test::foo @ bci:1 (line 62) !jvms: Integer::valueOf @ bci:23 (line 1074) Test::foo @ bci:1 (line 62) LocalVar(10) [ 153P [ 170 228b ]] 165 Proj === 153 [[ 166 170 228 ]] #5 !jvms: Integer::valueOf @ bci:23 (line 1074) Test::foo @ bci:1 (line 62) LocalVar(12) [ 165 153P [ 94 ]] 170 CheckCastPP === 167 165 [[ 234 94 ]] #java/lang/Integer:NotNull:exact * Oop:java/lang/Integer:NotNull:exact * !jvms: Integer::valueOf @ bci:23 (line 1074) Test::foo @ bci:1 (line 62) LocalVar(13) [ 150 170 1P 153P [ ]] 94 Phi === 90 150 170 [[ 50 ]] #java/lang/Integer:NotNull:exact * Oop:java/lang/Integer:NotNull:exact * !jvms: Test::foo @ bci:1 (line 62) with -XX:+AggressiveLivenessForUnstableIf, PHI is dead. as a result, Integer.valueOf(value) is dead then. c2 deletes it in late inliner.
05-05-2022
A pull request was submitted for review. URL: https://git.openjdk.java.net/jdk/pull/8545 Date: 2022-05-05 05:30:06 +0000
05-05-2022
I linked JDK-8276998 and converted this to an enhancement.
04-05-2022