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

Thu Mar 7 17:57:19 UTC 2019

Hi Vladimir,

thank you very much for reviewing.

Best regards,
Martin

-----Original Message-----
From: Vladimir Kozlov <vladimir.kozlov at oracle.com> 
Sent: Donnerstag, 7. März 2019 18:50
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

On 3/7/19 7:38 AM, Doerr, Martin wrote:
> 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?

Yes,  uncommon traps was my concern. That is why I asked if you looked on generated code.

> 
> 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.

Yes, it looks good!

> 
>> 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?

I agree since generated code look good.

> 
>> 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.

I was concern that you may get a timeout due to long loop if it is not interrupted. But since you set times to 0.5 sec 
it could happens only if we have a bug. So it seems good.

> 
> Can I add you as reviewer?

Yes. I looked on changes and they are good

Thanks,
Vladimir

> 
> 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
>>