Thank you, Jc!
Serguei

On 4/9/19 2:21 PM, Jean Christophe Beyler wrote:
Hi Serguei,

Looks good to me now :)
Jc

On Tue, Apr 9, 2019 at 1:58 PM <[email protected] <mailto:[email protected]>> wrote:

    Hi Jc,

    Thank you a lot for looking at this!

    On 4/9/19 9:29 AM, Jean Christophe Beyler wrote:
    Hi Serguei,

    I saw a nit here:

    
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8222072-jvmti-GenerateEvents.1/test/hotspot/jtreg/serviceability/jvmti/GenerateEvents/MyPackage/GenerateEventsTest.java.html
    
<http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2019/8222072-jvmti-GenerateEvents.1/test/hotspot/jtreg/serviceability/jvmti/GenerateEvents/MyPackage/GenerateEventsTest.java.html>

    arei -> are

    Nice catch, fixed.

    - I'm not sure you needed two files for the agents, you could
    have re-used some of the code and just called twice GetEnv but
    that is a detail.

    Right.
    I initially wanted to use this way but then decided to use two
    real agent libraries.
    Wanted to exercise this path.

    - I thought JNIEnv was not supposed to be really kept because it
    should not be used by another thread, is there not a risk that
    you are doing that? It doesn't look like it but I've been
    surprised in the past

    You are right.
    Tried to fix it in new webrev version below.


    - Isn't there a chance that your second agent gets a normal
    JVMTI_EVENT_COMPILED_METHOD_LOAD before the GenerateEvents call
    and increments its counter?
       - I guess we'd see if it becomes flaky at some point? :)
    Yes, it is expected.
    But they should be posted on threads other than Main thread.
    The callback should ignore them.


    The updated fix version is:
    
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8222072-jvmti-GenerateEvents.2/
    
<http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2019/8222072-jvmti-GenerateEvents.2/>


    Thanks!
    Serguei


    Thanks!
    Jc


    On Mon, Apr 8, 2019 at 12:29 PM [email protected]
    <mailto:[email protected]> <[email protected]
    <mailto:[email protected]>> wrote:

        Please, review a fix for:
        https://bugs.openjdk.java.net/browse/JDK-8222072

        Webrev:
        
http://cr.openjdk.java.net/~sspitsyn/webrevs/2019/8222072-jvmti-GenerateEvents.1/
        
<http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2019/8222072-jvmti-GenerateEvents.1/>


        Summary:
          The JVMTI GenerateEvents() must send CompiledMethodLoad
        events only to the agent which called it.
          However, it sends events to all agents (jvmti
        environements) which violates the JVMTI spec.
          The webrev above fixes this issue and adds new jtreg test:
        test/hotspot/jtreg/serviceability/jvmti/GenerateEvents


        Testing:
          Mach5 submission for:
           - JVMTI tests: open/test/hotspot/jtreg/vmTestbase/nsk/jvmti
        - new test:
        test/hotspot/jtreg/serviceability/jvmti/GenerateEvents

        Thanks,
        Serguei



--
    Thanks,
    Jc



--

Thanks,
Jc

Reply via email to