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
>
>
