Thank you, Dean and Chris, for reviewing this change!
--Daniil
On 3/4/19, 7:00 PM, "Chris Plummer" <[email protected]> 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, "[email protected] on
behalf of [email protected]" <[email protected] on
behalf of [email protected]> 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
> >
> >
>
>
>
>
>
