8209585: [Graal] vmTestbase/nsk/jvmti/scenarios/sampling tests fail with "Too small stack of resumed thread"

[Daniil Titov]

Hi Jc,

 

Thank you for reviewing this change. I had the similar concerns initially regarding the losing coverage in the general case and was thinking about detecting the Graal frames and adjusting the effective stack depth to not count them. However, I found this approach a quite cumbersome and finally decided to not follow it.  In the approach you suggested my concern is that even for consequent calls of GetFrameCount(), as I understand, there is no guarantee that there are no Graal frames on the stack and it would result in the test failure.

 

Best regards,

Daniil

 

From: JC Beyler <jcbeyler at google.com>
Date: Friday, August 24, 2018 at 4:49 PM
To: <daniil.x.titov at oracle.com>
Cc: <serviceability-dev at openjdk.java.net>
Subject: Re: RFR: 8209585: [Graal] vmTestbase/nsk/jvmti/scenarios/sampling tests fail with "Too small stack of resumed thread"

 

Hi Daniil,

 

Just my two cents about this :)

 

I was looking at this and wondered if it made sense to fix the test this way (I always worry about simplifying a test and losing coverage). I understand the bug is that it is possible that between both calls, Graal could add some frames and therefore might trip this test:

 

-        if (frameStackSize < frameCount) {

 

However, by removing the test altogether and only relying on the suspended frames, are we not reducing our coverage of the test (basically never really testing the running threads anymore, only the suspended ones?).

 

Alternatively, when we look at this code and the hypothesis of Graal stacks "slipping in between calls", two cases could occur:

  A) The Graal frames are present in the first call but not the second

  B) The Graal frames are present in the second call but not the first

 

In the (B) case, the test would not trip, as frameStackSize would be >= frameCount so that is not an issue.

In the (A) case, we could simply recall the frameCount and assure ourselves the frames have disappeared, no?

 

Something like:

 

        if (frameStackSize < frameCount) {

           // This can occur for Graal if graal frames crept in. Call getFrameCount again and see if they have disappeared since

          // frameStackSize seems to say so.

           ... insert call here and a new check...

 

            NSK_COMPLAIN5("Too small stack of %s thread #%d (%s):\n"

                            "#   getStackTrace(): %d\n"

                            "#   getFrameCount(): %d\n",

                            kind, i, threadsDesc[i].threadName,

                            (int)frameStackSize, (int)frameCount);

            nsk_jvmti_setFailStatus();

        }

 

Just my 2 cents because I worry about simplifying a test for Graal but losing coverage in the general case,

Jc

 

 

 

On Thu, Aug 23, 2018 at 8:29 PM Daniil Titov <daniil.x.titov at oracle.com> wrote:

Please review the change that fixes 4 JVMTI tests when running with Graal.

One of the checks these tests perform compares the number of frames in the thread's stack returned by JVMTI GetFrameCount() with the number of frames entries returned by JVMTI GetStackTrace(). The thread to be tested executes arithmetic operations in the loop so the consequent calls of GetFrameCount() or/and  GetStackTrace() should return the stack trace of the same depth.

However,  with Graal on, additional "adjustCompilationLevel" frames could appear on the stack trace, e.g.:

adjustCompilationLevel:166, HotSpotGraalCompilerFactory (org.graalvm.compiler.hotspot)
adjustCompilationLevel:504, HotSpotJVMCIRuntime (jdk.vm.ci.hotspot)
testedMethod:56, Test$Runner
run:67, Test$Runner

that results in the stack depth reported by the first invocation of GetFrameCount() or GetStackTrace() might differ from the stack depth reported by the consequent calls of the same methods.

The fix modifies the tests to ensure the check that GetFrameCount () and GetStackTrace() report the same stack depth is performed only if the thread is suspended. For two tests (vmTestbase/nsk/jvmti/scenarios/sampling/SP02/sp02t001/TestDescription.java  and vmTestbase/nsk/jvmti/scenarios/sampling/SP06/sp06t001/TestDescription.java) such check for suspended threads already exists so in these tests the problematic check was not modified by just removed.

Issue: https://bugs.openjdk.java.net/browse/JDK-8209585 
Webrev: http://cr.openjdk.java.net/~dtitov/8209585/webrev.01

Thanks,
Daniil



 

-- 

 

Thanks,

Jc

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20180827/9924ed7d/attachment.html>