Request for reviews (M) 8005600: compiler/8004741/Test8004741.java fails intermediately

[Vladimir Kozlov]

On 1/21/13 3:47 PM, David Chase wrote:
>
> On 2013-01-21, at 6:17 PM, Vladimir Kozlov <vladimir.kozlov at oracle.com> wrote:
>
>> Thank you, David
>>
>> First, this test tries to exercise paths in runtime code (not in compiled code) which pass exceptions to compiled code. 2 places are:
>>
>> 1. runtime calls from compiled methods (in opto/runtime.cpp). The test call it because variable values (passed as arguments, so it does not matter what values are passed) are used for multidimensional array allocation and flag -XX:+StressCompiledExceptionHandlers to avoid deoptimization of compiled frame which we do normally.
>
>> 2. during safepoint in JavaThread::send_thread_stop(). The test tries to bring VM into safepoint by allocating big array with small heap.
>
> How do we know whether or not we "hit" both places?  Is this a case where varying the allocation size would help?  I also found it interesting that I could see failures with small allocation under jdk7u11 -- a 2x2 allocation was adequate.

With current code we can't guaranty it. We can add an other @run command 
to the test to increase frequency of safepoints:

   * @run main/othervm -Xmx64m -Xbatch -XX:+IgnoreUnrecognizedVMOptions 
-XX:-TieredCompilation -XX:+StressCompiledExceptionHandlers Test8004741
+ * @run main/othervm -Xmx64m -Xbatch -XX:+IgnoreUnrecognizedVMOptions 
-XX:-TieredCompilation -XX:+StressCompiledExceptionHandlers 
-XX:+SafepointALot -XX:GuaranteedSafepointInterval=100 Test8004741

The test will be executed 2 times for each set of flags.

>
>> I think this cases should be separated (separate test methods). Also it would be nice to add third case, as you suggested, by injected invalid dimension size.
>
> I tried that; it did not trigger any crash under jdk7u11.  Do we try it anyway?

Yes, it will be test for future compiler source code changes (to avoid 
breaking exception catching in compiled code).

In 7u11, which does not have 8004741 fix, we always deoptimize compiled 
frame and Interpreter will catch any exception/error. So with 7u11 you 
are testing Interpreter instead of compiled code. It could be used to 
verify correctness of the test itself.

>
>> An other thing to improve is to add COMPILED state which set after 11000 iterations in the loop in run(). We test without tiered so threshold is 10000 for C2 (1000 for c1) and we use -Xbatch so the code will wait compilation finish.
>
> The state variable is per-thread, tracking that thread's progress.  COMPILED ought to be true for all of them after the warm-up loop, and I don't know how to verify that the compiler has run; I could make up a compiled state and set it, but I no way of knowing that the code was in fact compiled when I set the state to COMPILED.

Correct, we can't guaranty it but it is more accurate then rely on time. 
I want main thread wait for COMPILED instead of RUNNING. Actually we can 
set RUNNING after 11000 iterations then you don't need to change code in 
main().

>
>> It would be nice to avoid (passed > N/2) condition to pass test. Is it possible to find why it fails?
>
> I think that there are other safepoints besides the one at the array allocation; for example, the test() method is called from within an endless loop.  That loop might have a safepoint in it; the method entry (before the try-catch is entered) might have a safepoint in it.  If ThreadDeath lands on either of those non-allocation safepoints, we haven't really tested for the regression.  I don't think there is any easy way to get rid of those other safepoints -- not easier than making the test statistical.
>
> In the old test it was not possible to tell if this had happened because the loop itself was with in a try-catch that caught ThreadDeath and set "passed"; however, because it used timing to guess at thread progress, in some cases it apparently missed for other reasons.  I decided to tighten up the test case, and deal with the might-miss nature by making it statistical.  I experimentally added an array allocation for padding outside the try-catch, and it was easy to miss more than half of the ThreadDeath exceptions.
>

Yes, you are right. There is also SFP on the return from test() which is 
outside of try block. And it is impossible to tell difference between 
missing the catch in test() method because bug in compiled code or 
because exception happened outside the try block.

OK, leave statistical pass.

Thanks,
Vladimir

>
>
>> Thanks,
>> Vladimir
>>
>> On 1/19/13 7:05 AM, David Chase wrote:
>>>
>>> http://cr.openjdk.java.net/~drchase/8006500/webrev.01/
>>>
>>> 8004741 tested for the multi-dimensional-array-allocation-lack-exception-handler bug.
>>> This is accomplished by allocating a multi-dimensional array in an infinite loop,
>>> Thread.stop()ing the looping allocator (asynchronously throws ThreadDeath in victim thread).
>>>
>>> The old logic used timed waits to coordinate the two threads, and that did not always work;
>>> sometimes the ThreadDeath would land in an unexpected place.
>>> It also contained 6 seconds of total wait time.
>>>
>>> Fix:
>>>
>>> Rewrote most of the test surrounding the victim try-block.
>>>
>>> New version uses wait-notify to coordinate the two threads.
>>>
>>> The region of code in which a "pass" could be recorded was reduced.
>>>
>>> Pass was turned into a count, and the test was made statistical;
>>> if more than half of the N runs of the test code record a "pass" then
>>> it is judged to be a pass, else it is a fail.  The regression failure
>>> is a crash, so less than 100% testing is okay.
>>>
>>> All unexpected behavior (any exception other than ThreadDeath,
>>> and unexpected thread state changes) leads to a fail().
>>>
>>> The new version contains .5 + .1 * N seconds of explicit wait time
>>> (N=12, so 1.7), plus whatever is required for wait-notify thread coordination
>>> (ought to be milliseconds in the usual case).  The first .5 second might
>>> be completely unnecessary; I expect someone will tell me.
>>>
>>> Testing:
>>>
>>> Switching back and forth between pre-8004741-fix 7u11 and post-fix, verified that the modified 8004741 would still reliably fail on an unfixed VM.  Experimented with different sizes of 2-D allocation; that seemed to matter.
>>>
>>> Artificially padded the body of test to see that the statistical nature of the test would work -- that the try-block hit rate would go down (it did).
>>>
>>> JPRT of the new 8004741 on Mac, Solaris, x86 (some problem with running on Linux)
>>>
>>> - Checked that the x86 failures were all bogus (zip failure post-test; tests all passed)
>>> - Checked the logs of all the Mac and Solaris JPRT runs to look for any misses of the try-block; some Solaris runs saw one out of 12 trials miss once (i.e., 11 hits).  Note that this is hitting a "smaller target" than the previous version of the test, so these misses don't correspond to failures in the previous version of the test; that was due to timing hiccups.
>>>
>>>
>>>
>