JDK-8146096 : [TEST BUG] compiler/loopopts/UseCountedLoopSafepoints.java Timeouts
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 9
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2015-12-23
  • Updated: 2016-09-29
  • Resolved: 2016-09-01
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.
JDK 9
9 b138Fixed
Related Reports
Relates :  
JDK-6869327 - Add new C2 flag to keep safepoints in counted loops.
Relates :  
JDK-8166374 - compiler/loopopts/UseCountedLoopSafepointsTest.java fails with "Safepoint not found"
Sub Tasks
JDK-8148161 :  
quarantine compiler/loopopts/UseCountedLoopSafepoints.java - Resolved
Description
compiler/loopopts/UseCountedLoopSafepoints.java timeouts

first failure

options used:-Xcomp -client -XX:MaxRAMFraction=8 -XX:+CreateCoredumpOnCrash -ea -esa -XX:-TieredCompilation -XX:CompileThreshold=100 -XX:+UnlockExperimentalVMOptions -XX:+IgnoreUnrecognizedVMOptions -XX:+DeoptimizeALot

second failure

option used: -Xcomp -client -XX:MaxRAMFraction=8 -XX:+CreateCoredumpOnCrash -ea -esa -XX:-TieredCompilation -XX:CompileThreshold=100 -XX:+UnlockExperimentalVMOptions
Comments
Note, calls are safepoints. Use some code instead of calling addAndGet(). Or use one which is intrinsics. May be my suggestion about -Inline is wrong.
31-08-2016
Yes, it is similar to what I suggested with new WB API. There could be several safepoints in compiled method. If you can identify correct one - it may work. Also use -Xbatch and -XX:-Inline to make sure Ideal graph is simple.
30-08-2016
I am not against marking test as stress but I think the test itself is not good. It should be rewritten. There was discussion about it during original Andreas' RFR: nmethod has flags which record some compilation information: // set during construction unsigned int _has_unsafe_access:1; // May fault due to unsafe access. unsigned int _has_method_handle_invokes:1; // Has this method MethodHandle invokes? unsigned int _lazy_critical_native:1; // Lazy JNI critical native unsigned int _has_wide_vectors:1; // Preserve wide vectors at safepoints I thought about extending it. It could be difficult to justify adding a flag just for testing but if we add it only for debug VM it should be fine. Do we do WB testing only with [fast]debug VM? Anyway, I agree with your exception after loop - changes are fine now. Reviewed. Thanks, Vladimir On 2/17/16 5:15 AM, Andreas Eriksson wrote: > Hi, > > On 2016-02-16 20:45, Vladimir Kozlov wrote: >> So you implemented Vladimir Ivanov's suggestion. > > Yes, and also added whitebox API calls to make sure that the method is > C2 compiled, so checking the call stack shouldn't be necessary. > > Regarding your suggestion on a new WB interface: > I wasn't sure on how to do use WB to check that a SafePointNode was > generated. It looks to me like WB isn't hooked in to the actual > compilation steps, but instead IsMethodCompiled etc. just lookup fields > on the nmethod, at a point where we don't have access to the Ideal graph > anymore. So adding an interface to actually hook into the Ideal graph to > check for a SafePointNode seemed like a big investment for this test. > If I've misunderstood what you meant, or if there actually is an easy > way to do this, please let me know. > >> Okay but you should add a check that it did not finish the loop by >> checking _num value. Imaging very fast machine that will take less >> then 2 sec to finish loop without safepoint. > > Changed it so it throws an exception if the loop is ever finished. > New webrev: http://cr.openjdk.java.net/~aeriksso/8146096/webrev.01/ > Diff: http://cr.openjdk.java.net/~aeriksso/8146096/webrev.00.to.01/ > > Regards, > Andreas > >> >> Thanks, >> Vladimir >> >> On 2/16/16 6:24 AM, Andreas Eriksson wrote: >>> Hi, >>> >>> I've changed the test so it will not timeout, by using the >>> WhiteBox.forceSafepoint() in a separate thread. >>> If there is a safepoint in the loop, then the test will exit after >>> taking one safepoint. >>> If instead all safepoints in the loop were removed, then the safepoint >>> operation will timeout after two seconds (because of >>> -XX:+SafepointTimeout) and it will be detected and the test will fail. >>> >>> Webrev: http://cr.openjdk.java.net/~aeriksso/8146096/webrev.00/ >>> >>> Regards, >>> Andreas >>> >>> On 2016-01-20 10:26, Andreas Eriksson wrote: >>>> Vladimir Kozlov and Vladimir Ivanov, >>>> >>>> Ok, I'll look into using the whitebox api to fix the test. >>>> Thanks for looking at this. >>>> >>>> - Andreas >>>> >>>> On 2016-01-19 21:46, Vladimir Kozlov wrote: >>>>> Simple use timeout to check for generated safepoint is bad idea. It >>>>> is very inaccurate. At least you need to check call stack to see if >>>>> it stopped in compiled method. >>>>> I would prefer to see WB new interface which would check that loop >>>>> SafePointNode is generated during compilation of method. It will be >>>>> precise. >>>>> >>>>> And we need such tests to make sure a feature is working - we can't >>>>> remove them. >>>>> >>>>> Thanks, >>>>> Vladimir >>>>> >>>>> On 1/19/16 4:50 AM, Vladimir Ivanov wrote: >>>>>> As an idea to improve the test: spawn a thread which executes the >>>>>> counted loop and then use WhiteBox.forceSafepoint() to >>>>>> trigger a safepoint. >>>>>> >>>>>> If the test times out, it means there's no safepoint in the loop. >>>>>> >>>>>> Also, it also simplifies the implementation - no need to spawn a >>>>>> child process, the check can be done in-process. >>>>>> >>>>>> Best regards, >>>>>> Vladimir Ivanov >>>>>> >>>>>> On 1/19/16 3:32 PM, Andreas Eriksson wrote: >>>>>>> Hi, >>>>>>> >>>>>>> Can I please have a review for the removal of >>>>>>> hotspot/test/compiler/loopopts/UseCountedLoopSafepoints.java. >>>>>>> >>>>>>> The test needs to do a loop that takes more than two seconds to >>>>>>> execute >>>>>>> fully without doing a safepointing call. For this expensive atomic >>>>>>> operations were used. The problem is that on certain embedded >>>>>>> platforms >>>>>>> they are too expensive, and the test times out. >>>>>>> The loop length could probably be reduced, and it should still >>>>>>> work on >>>>>>> faster machines. However, the test is not very useful, so I think >>>>>>> it's >>>>>>> better to just remove it to avoid future problems. >>>>>>> >>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8146096 >>>>>>> Test to be removed: >>>>>>> http://hg.openjdk.java.net/jdk9/hs-comp/hotspot/file/d84a55e7aaf8/test/compiler/loopopts/UseCountedLoopSafepoints.java >>>>>>> >>>>>>> >>>>>>> >>>>>>> (I can create a webrev if you think it necessary.) >>>>>>> >>>>>>> Thanks, >>>>>>> Andreas >>>> >>> >
29-08-2016
Since test is "heavy" for some platforms (especially with "-Xcomp -XX:+DeoptimizeALot" option), it should be modified to stop iterations after some time. Also, taking iterations count and execution time into account, this test should be marked as "stress".
29-08-2016
Hi Andreas, who do you expect will do the rework? Please assign the bug to that person. Thank you! Zoltan
30-05-2016
Couldn't get the test to work reliably in all cases, might need a bigger rework.
27-05-2016
The whitebox api was suggested as a potential way to fix it, looking into that.
21-01-2016
The test needs to do a loop that takes more than two seconds to execute fully. For this expensive atomic operations were used. Looks like they are too expensive on that platform, and the test times out. The loop length could probably be reduced and it would still work on faster machines. However, the test is not super useful, so it might be better to just remove it to avoid future problems.
19-01-2016
Hi Andreas, you've added the test that timeouts. Could you please investigate what causes the timeout? Thanks, Zoltan
18-01-2016
There are timing assumptions in test case, test will behave differently in different deployment, validation and exit checks are not proper. so on slow machines test timeout!
08-01-2016