[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-serviceability-dev
Subject:    Re: RFR: 8153749 - New capability can_generate_early_class_hook_events
From:       "serguei.spitsyn () oracle ! com" <serguei ! spitsyn () oracle ! com>
Date:       2016-05-25 1:09:20
Message-ID: 5744FB40.1020708 () oracle ! com
[Download RAW message or body]

On 5/24/16 17:24, Daniel D. Daugherty wrote:
> On 5/24/16 6:03 PM, serguei.spitsyn@oracle.com wrote:
>> On 5/24/16 16:23, Daniel D. Daugherty wrote:
>>> > Testing:
>>> >    Altered the nsk.jvmti co-located test 
>>> nsk/jvmti/ClassFileLoadHook/classfloadhk002
>>> >    to enable the can_generate_early_class_hook_events and checked 
>>> that new CFLH events
>>> >    are posted in the primordial phase and also they are not posted 
>>> otherwise.
>>>
>>> Sorry for looping back on this...
>>>
>>> I've been re-reading this thread off and on for a while now...
>>>
>>> I'm okay with this part of the testing statement:
>>>
>>> > checked that new CFLH events are posted in the primordial phase
>>>
>>> but this part bothers me:
>>>
>>> > and also they are not posted otherwise
>>>
>>> I'm not sure what that last part means exactly. I think it might
>>> be saying that there are certain CFLH events that the test expects
>>> to be posted in the primordial phase and the test verifies that
>>> those CFLH events are posted in the primordial phase and not in
>>> the next phase (the start phase). I'm hoping that the testable
>>> assertion is more clear than the above text.
>>
>> Dan, sorry that my statement above is not clear.
>> The 'otherwise' means the same test but unaltered.
>> Unaltered test does not enable new capability and so, the CFLH events
>> should not be posted in the primordial phase.
>
> Got it. I'm good with a test to verify that the older JVM/TI
> behavior works as expected along with a test for the newer
> behavior.
>
>
>>
>>>
>>> One other possible issue: if the primordial phase is still
>>> single threaded, then there is no race between a thread doing
>>> this early class loading (and posting of the CFLH events) and
>>> the thread that is changing the JVM/TI phase from "primordial
>>> phase" to "start phase". However, if the primordial phase is
>>> now multi-threaded, then there might be a race between the
>>> thread posting the CFLH event and JVM/TI phase switch...
>>
>> This is interesting observation, thanks.
>> Why do you think the primordial thread is multi-threaded now?
>
> I haven't been paying close attention to Jigsaw so I'm not sure
> if primordial phase is multi-threaded or not...
I can be wrong but ...
My current understanding is that it is still single-threaded
as I did not see any discussions or related changes in the code.

>
>
>> We have this kind of race problem in general, especially
>> with the JVM/TI phase switch to the DEAD phase.
>> The phase can be switched even in the middle of the event post.
>> I'm currently looking at several bugs related to 
>> JVMTI_ERROR_WRONG_PHASE(112).
>> This problem needs a good solution, at least to make the nightly 
>> testing stable.
>
> Yes, I'm well aware of that problem. I believe I've made a number
> of comments on various JVMTI_ERROR_WRONG_PHASE bugs over the years.
> It's going to need some sort of handshake to solve it. Much like
> the suspend/resume dance that we do on thread exit...
>
> We'll have to brainstorm on how to make things more robust...

Great, thanks!
I've already started thinking on this. :)


Thanks,
Serguei

>
> Dan
>
>
>>
>>
>> Thanks,
>> Serguei
>>
>>
>>>
>>> Dan
>>>
>>>
>>> On 4/14/16 2:24 AM, serguei.spitsyn@oracle.com wrote:
>>>> Please, review the Jigsaw-related fix for:
>>>> https://bugs.openjdk.java.net/browse/JDK-8153749
>>>>
>>>>
>>>> Hotspot webrev:
>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8153749-Jigsaw-newcap.hs1/ 
>>>>
>>>>
>>>> Jdk webrev:
>>>> http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/jdk/8153749-Jigsaw-newcap.jdk1/ 
>>>>
>>>>
>>>>
>>>> Summary:
>>>>
>>>>    This is a Jigsaw related enhancement.
>>>>    Some agents need to get a CFLH event for classes loaded in the 
>>>> primordial phase.
>>>>    This is not possible in JDK 9 because existing agents may 
>>>> instrument code in the
>>>>    primordial or start phase before the module system has completed 
>>>> initialization.
>>>>
>>>>    We introduce a new capability: 
>>>> can_generate_early_class_hook_events.
>>>>    If this capability and can_generate_all_class_hook_events are 
>>>> enabled then
>>>>    the CFLH event could be posted for classes loaded in the 
>>>> primordial phase.
>>>>    We leave can_generate_early_vmstart as is, no changes.
>>>>
>>>>    This enhancement needs a CCC request filed.
>>>>    I will file it once the JVMTI spec changes are reviewed.
>>>>
>>>>
>>>> Testing:
>>>>    Altered the nsk.jvmti co-located test 
>>>> nsk/jvmti/ClassFileLoadHook/classfloadhk002
>>>>    to enable the can_generate_early_class_hook_events and checked 
>>>> that new CFLH events
>>>>    are posted in the primordial phase and also they are not posted 
>>>> otherwise.
>>>>
>>>>
>>>> Thanks,
>>>> Serguei
>>>>
>>>
>>
>


[Attachment #3 (text/html)]

<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <div class="moz-cite-prefix">On 5/24/16 17:24, Daniel D. Daugherty
      wrote:<br>
    </div>
    <blockquote
      cite="mid:c8d62d04-a45b-c5db-6c27-3bde7f335fb4@oracle.com"
      type="cite">
      <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
      On 5/24/16 6:03 PM, <a moz-do-not-send="true"
        class="moz-txt-link-abbreviated"
        href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a>
      wrote:<br>
      <blockquote cite="mid:5744EBC6.9070105@oracle.com" type="cite">
        <meta content="text/html; charset=utf-8"
          http-equiv="Content-Type">
        <div class="moz-cite-prefix">On 5/24/16 16:23, Daniel D.
          Daugherty wrote:<br>
        </div>
        <blockquote
          cite="mid:0d89333b-8534-ae3c-ee85-86c5ab3e85d9@oracle.com"
          type="cite">&gt; Testing: <br>
          &gt;       Altered the nsk.jvmti co-located test
          nsk/jvmti/ClassFileLoadHook/classfloadhk002 <br>
          &gt;       to enable the can_generate_early_class_hook_events and
          checked that new CFLH events <br>
          &gt;       are posted in the primordial phase and also they are
          not posted otherwise. <br>
          <br>
          Sorry for looping back on this... <br>
          <br>
          I've been re-reading this thread off and on for a while now...
          <br>
          <br>
          I'm okay with this part of the testing statement: <br>
          <br>
          &gt; checked that new CFLH events are posted in the primordial
          phase <br>
          <br>
          but this part bothers me: <br>
          <br>
          &gt; and also they are not posted otherwise <br>
          <br>
          I'm not sure what that last part means exactly. I think it
          might <br>
          be saying that there are certain CFLH events that the test
          expects <br>
          to be posted in the primordial phase and the test verifies
          that <br>
          those CFLH events are posted in the primordial phase and not
          in <br>
          the next phase (the start phase). I'm hoping that the testable
          <br>
          assertion is more clear than the above text. <br>
        </blockquote>
        <br>
        Dan, sorry that my statement above is not clear.<br>
        The 'otherwise' means the same test but unaltered.<br>
        Unaltered test does not enable new capability and so, the CFLH
        events<br>
        should not be posted in the primordial phase.<br>
      </blockquote>
      <tt>  <br>
        Got it. I'm good with a test to verify that the older JVM/TI<br>
        behavior works as expected along with a test for the newer<br>
        behavior.<br>
        <br>
        <br>
      </tt>
      <blockquote cite="mid:5744EBC6.9070105@oracle.com" type="cite"> <br>
        <blockquote
          cite="mid:0d89333b-8534-ae3c-ee85-86c5ab3e85d9@oracle.com"
          type="cite"> <br>
          One other possible issue: if the primordial phase is still <br>
          single threaded, then there is no race between a thread doing
          <br>
          this early class loading (and posting of the CFLH events) and
          <br>
          the thread that is changing the JVM/TI phase from "primordial
          <br>
          phase" to "start phase". However, if the primordial phase is <br>
          now multi-threaded, then there might be a race between the <br>
          thread posting the CFLH event and JVM/TI phase switch...<br>
        </blockquote>
        <br>
        This is interesting observation, thanks.<br>
        Why do you think the primordial thread is multi-threaded now?<br>
      </blockquote>
      <tt>  <br>
        I haven't been paying close attention to Jigsaw so I'm not sure<br>
        if primordial phase is multi-threaded or not...<br>
      </tt></blockquote>
    I can be wrong but ...<br>
    My current understanding is that it is still single-threaded<br>
    as I did not see any discussions or related changes in the code.<br>
    <br>
    <blockquote
      cite="mid:c8d62d04-a45b-c5db-6c27-3bde7f335fb4@oracle.com"
      type="cite"><tt> <br>
        <br>
      </tt>
      <blockquote cite="mid:5744EBC6.9070105@oracle.com" type="cite"> We
        have this kind of race problem in general, especially<br>
        with the JVM/TI phase switch to the DEAD phase.<br>
        The phase can be switched even in the middle of the event post.<br>
        I'm currently looking at several bugs related to
        JVMTI_ERROR_WRONG_PHASE(112).<br>
        <meta http-equiv="content-type" content="text/html;
          charset=utf-8">
        This problem needs a good solution, at least to make the nightly
        testing stable.<br>
      </blockquote>
      <tt>  <br>
        Yes, I'm well aware of that problem. I believe I've made a
        number<br>
        of comments on various JVMTI_ERROR_WRONG_PHASE bugs over the
        years.<br>
        It's going to need some sort of handshake to solve it. Much like<br>
        the suspend/resume dance that we do on thread exit...<br>
        <br>
        We'll have to brainstorm on how to make things more robust...<br>
      </tt></blockquote>
    <br>
    Great, thanks!<br>
    I've already started thinking on this. :)<br>
    <br>
    <br>
    Thanks,<br>
    Serguei<br>
    <br>
    <blockquote
      cite="mid:c8d62d04-a45b-c5db-6c27-3bde7f335fb4@oracle.com"
      type="cite"><tt> <br>
        Dan<br>
        <br>
        <br>
      </tt>
      <blockquote cite="mid:5744EBC6.9070105@oracle.com" type="cite"> <br>
        <br>
        Thanks,<br>
        Serguei<br>
        <br>
        <br>
        <blockquote
          cite="mid:0d89333b-8534-ae3c-ee85-86c5ab3e85d9@oracle.com"
          type="cite"> <br>
          Dan <br>
          <br>
          <br>
          On 4/14/16 2:24 AM, <a moz-do-not-send="true"
            class="moz-txt-link-abbreviated"
            href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a>
          wrote: <br>
          <blockquote type="cite">Please, review the Jigsaw-related fix
            for: <br>
               <a moz-do-not-send="true" class="moz-txt-link-freetext"
              href="https://bugs.openjdk.java.net/browse/JDK-8153749">https://bugs.openjdk.java.net/browse/JDK-8153749</a>
  <br>
            <br>
            <br>
            Hotspot webrev: <br>
            <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2016/hotspot/8153749-Jigsaw-newca \
p.hs1/">http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/hotspot/8153749-Jigsaw-newcap.hs1/</a>
  <br>
            <br>
            Jdk webrev: <br>
            <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Esspitsyn/webrevs/2016/jdk/8153749-Jigsaw-newcap.jd \
k1/">http://cr.openjdk.java.net/~sspitsyn/webrevs/2016/jdk/8153749-Jigsaw-newcap.jdk1/</a>
  <br>
            <br>
            <br>
            Summary: <br>
            <br>
                 This is a Jigsaw related enhancement. <br>
                 Some agents need to get a CFLH event for classes loaded
            in the primordial phase. <br>
                 This is not possible in JDK 9 because existing agents may
            instrument code in the <br>
                 primordial or start phase before the module system has
            completed initialization. <br>
            <br>
                 We introduce a new capability:
            can_generate_early_class_hook_events. <br>
                 If this capability and can_generate_all_class_hook_events
            are enabled then <br>
                 the CFLH event could be posted for classes loaded in the
            primordial phase. <br>
                 We leave can_generate_early_vmstart as is, no changes. <br>
            <br>
                 This enhancement needs a CCC request filed. <br>
                 I will file it once the JVMTI spec changes are reviewed.
            <br>
            <br>
            <br>
            Testing: <br>
                 Altered the nsk.jvmti co-located test
            nsk/jvmti/ClassFileLoadHook/classfloadhk002 <br>
                 to enable the can_generate_early_class_hook_events and
            checked that new CFLH events <br>
                 are posted in the primordial phase and also they are not
            posted otherwise. <br>
            <br>
            <br>
            Thanks, <br>
            Serguei <br>
            <br>
          </blockquote>
          <br>
        </blockquote>
        <br>
      </blockquote>
      <br>
    </blockquote>
    <br>
  </body>
</html>



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic