Hi Chris,
I also was wondering about what might be behind the decision to use
"commonDepth" when the tests were originally written. You are right there were
several issues in these tests when running with Graal. The first issue (the
different stack sizes returned by jvmti->GetFrameCount and jvmti->
GetStackTrace when Graal is on and the thread is not suspended) was addressed
by JDK-8051349 and another issue ( JDK- 8218167, that the test method is not
found in the frame stack) is addressed by this change.
Best regards,
Daniil
On 3/4/19, 7:00 PM, "Chris Plummer" <chris.plum...@oracle.com> wrote:
Hi Daniil,
I think the fix is fine.
I was wondering why the test was originally written to use the smaller
of the two stack sizes. I noticed that the "suspended == NSK_TRUE"
checks seem to defend against test failures that might be introduced
with your change (you can only get two different stack sizes when the
thread is not suspended). This check was added as part of the fix for
JDK-8051349, which was done a few months ago, so if the fix for
JDK-8051349 were not in place then your fix would not have worked.
thanks,
Chris
On 3/4/19 3:03 PM, Daniil Titov wrote:
> Hi Dean,
>
> You are right, test sp06t003 has the same problem. Please, review a new
version of the change that fixes both tests. I checked other tests and no more
tests use the this approach with "commonDepth".
>
> Webrev: http://cr.openjdk.java.net/~dtitov/8218167/webrev.02
> Bug: https://bugs.openjdk.java.net/browse/JDK-8218167
>
> Thanks!
> --Daniil
>
> On 3/1/19, 9:14 PM, "serviceability-dev-boun...@openjdk.java.net on
behalf of dean.l...@oracle.com" <serviceability-dev-boun...@openjdk.java.net on
behalf of dean.l...@oracle.com> wrote:
>
> Looks good, but what about sp06t003? Doesn't it have the same
problem?
> Are there any other tests using similar logic?
>
> dl
>
> On 3/1/19 8:33 PM, Daniil Titov wrote:
> > Please review the change that fix intermittent failure for test
nsk/jvmti/scenarios/sampling/SP02/sp02t003 when running with Graal.
> >
> > The problem with the test here is that method checkThread() looks
for the test method in the top "commonDepth" frames where "commonDepth" is a
minimum of "frameCount" (returned by jvmti->GetFrameCount) and
"frameStackSize"( returned by jvmti->GetStackTrace).
> >
> > If a compilation is triggered between these 2 calls then there are
cases when "frameCount" is 2, "frameStackSize" is 4, and the frame stack is
as the following:
> >
> > [0] adjustCompilationLevel
> > [1] adjustCompilationLevel
> > [2] testedMethod
> > [3] run
> >
> > In this case the test looks for the test method only in 2 top
frames and fails.
> >
> > The fix ensures that the test iterates over all frames in the
frame stack when looking for the test method.
> >
> > Webrev: http://cr.openjdk.java.net/~dtitov/8218167/webrev.01
> > Bug: https://bugs.openjdk.java.net/browse/JDK-8218167
> >
> > Thanks!
> > --Daniil
> >
> >
>
>
>
>
>
