RFR 8218167: nsk/jvmti/scenarios/sampling/SP02/sp02t003 fails

[Daniil Titov]

Thank you, Dean and Chris,  for reviewing this change!

--Daniil

    
    
    On 3/4/19, 7:00 PM, "Chris Plummer" <chris.plummer at 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-bounces at openjdk.java.net on behalf of dean.long at oracle.com" <serviceability-dev-bounces at openjdk.java.net on behalf of dean.long at 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
        >      >
        >      >
        >      
        >      
        >      
        >
        >