RFR(M): 8219584: Try to dump error file by thread which causes safepoint timeout

[Doerr, Martin]

Hi Vladimir,

thank you for reviewing it so quickly and for your good points.

> Compiling with -Xcomp may produce unexpected result.
In general, yes, but I didn't see any uncommon traps in this simple method.
Were you concerned about that?

I'm using -Xcomp + compileonly in order to get the test_loop method precompiled.
(Also avoids OnStackReplacement.)

> Did you look on generated code for test_loop() method?
Yes, looks good. Also see OptoAssembly from SPARC below.

> Also use something smaller then Integer.MAX_VALUE for limit (subtract -100 for example) to simplify logic for overflow 
> checks.
I've tried, but there was no change in OptoAssembly other than the constant value.
I've looked at the loopnode.cpp overflow checking logic and it appears to be implemented nicely and can detect that stride = 1 and limit "< max_jint" will not overflow.
Do you agree?

> You may also add -XX:LoopUnrollLimit=0 to avoid unrolling and other loop optimizations which you don't need. Check 
> generated code.
Good idea. Makes the OptoAssembly better readably. Added in place. Thanks.

> Did you tested on SPARC?
Of course. Works great on this platform. Also see OptoAssembly below.

> How long it takes to run on it?
Took less than a minute. Hmm... seems like the jtreg stuff takes most of the time.
The test itself shouldn't run much longer than 1 second (500ms GuaranteedSafepointInterval + 500ms SafepointTimeoutDelay) until the Java Thread gets killed.

Can I add you as reviewer?

Thanks and best regards,
Martin


OptoAssembly SPARC

000   B1: #     B5 B2 <- BLOCK HEAD IS JUNK   Freq: 1
000     ! stack bang (144 bytes)
        SAVE   R_SP,-144,R_SP
014
 MOV    R_I0,R_L3       ! spill
018 +   MOV    #0,R_I0
01c     CWBeq  R_L3,#0,B5       ! int  P=0.100000 C=-1.000000
01c
020   B2: #     B3 <- B1  Freq: 0.9
020 +   SET    #2147483647,R_L0
028 +   MOV    #1,R_L2
028
02c   B3: #     B6 B4 <- B2 B4       Loop: B3-B4 inner  Freq: 9
02c     SREM   R_L2,R_L3,R_L1
040 +   CWBeq  R_L1,#0,B6       ! int  P=0.100000 C=-1.000000
040
044   B4: #     B3 B5 <- B6 B3  Freq: 9
044 +   ADD    R_L2,#1,R_L2
048     CWBlt  R_L2,R_L0,B3     ! Loop end  P=0.900000 C=-1.000000
048
04c   B5: #     N1 <- B4 B1  Freq: 1
04c     LDX    [R_G2 + #poll_offset],L0 ! Load local polling address
        LDX    [L0],G0  !Poll for Safepointing
        RET
        RESTORE
05c +   ! return
05c
05c   B6: #     B4 <- B3  Freq: 0.9
05c +   ADD    R_I0,#1,R_I0
060     BA     B4       ! short branch



-----Original Message-----
From: Vladimir Kozlov <vladimir.kozlov at oracle.com> 
Sent: Mittwoch, 6. März 2019 20:14
To: Doerr, Martin <martin.doerr at sap.com>; 'hotspot-compiler-dev at openjdk.java.net' <hotspot-compiler-dev at openjdk.java.net>
Cc: David Holmes (david.holmes at oracle.com) <david.holmes at oracle.com>
Subject: Re: RFR(M): 8219584: Try to dump error file by thread which causes safepoint timeout

Hi Martin,

On 3/6/19 3:43 AM, Doerr, Martin wrote:
> Hi,
> 
> my proposal JDK-8219584 is currently being reviewed on hotspot-runtime-dev, but it contains a small test which 
> explicitly uses C2.
> 
> May I get a review for TestAbortVMOnSafepointTimeout.java, please?
> 
> Webrev:
> 
> http://cr.openjdk.java.net/~mdoerr/8219584_kill_thread_on_safepoint_timeout/webrev.02/
> 
> Bug with description of the feature:
> 
> https://bugs.openjdk.java.net/browse/JDK-8219584
> 
> The purpose of the method "test_loop" is to loop long enough to hit a safepoint timeout (with configured timeout delay).
> 
> I compile it directly by C2 with -XX:-UseCountedLoopSafepoints and -XX:LoopStripMiningIter=0.

Yes, these flags combination should work even if one of this flag is set by testing infra.
Also you correctly use @requires vm.compiler2.enabled to run VM build where these flag are available.

> 
> This should force the loop to get compiled without safepoint and 2 billion divisions should definitely take long enough 
> to hit a 500ms safepoint timeout.

Compiling with -Xcomp may produce unexpected result. Did you look on generated code for test_loop() method?
Also use something smaller then Integer.MAX_VALUE for limit (subtract -100 for example) to simplify logic for overflow 
checks.
You may also add -XX:LoopUnrollLimit=0 to avoid unrolling and other loop optimizations which you don't need. Check 
generated code.

> 
> I've tested it many times on all platforms we have and I've never seen it failing.

Did you tested on SPARC? How long it takes to run on it?

> 
> Is it fine to rely on this?

Yes, I think so.

Vladimir

> 
> Best regards,
> 
> Martin
>