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

List:       openjdk-serviceability-dev
Subject:    Re: RFR (M) 8221183: Avoid code cache walk in MetadataOnStackMark
From:       coleen.phillimore () oracle ! com
Date:       2019-03-29 21:53:49
Message-ID: 8714487a-c2a8-0b07-e7fb-7b43dbc2c090 () oracle ! com
[Download RAW message or body]

Serguei,
Thank you for reviewing.

On 3/29/19 3:06 PM, serguei.spitsyn@oracle.com wrote:
> Hi Coleen,
> 
> It looks good to me.
> I agree with Erik on reset_old_table().
> 
> I have one question.
> 
> http://cr.openjdk.java.net/~coleenp/2019/8221183.01/webrev/src/hotspot/share/code/codeCache.cpp.frames.html
>  

Yes, I changed it to reset_old_method_table.

> 1077 void CodeCache::mark_for_evol_deoptimization(InstanceKlass* 
> dependee) {
> *1078 MutexLockerEx mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);*
> 1079
> 1080   // Mark dependent AOT nmethods, which are only found via the class \
> redefined. 1081 // TODO: add dependencies to aotCompiledMethod's metadata section 
> so this isn't
> 1082 // needed.
> 1083   AOTLoader::mark_evol_dependent_methods(dependee);
> 1084 }
> 
> Is it still necessary to grab the CodeCache_lock here?
> 
It's actually not necessary because we're at a safepoint, and this code 
is always at a safepoint.

I'll assert it's at a safepoint.   flush_evol_dependents has this:

    // --- Compile_lock is not held. However we are at a safepoint.
    assert_locked_or_safepoint(Compile_lock);

Thanks,
Coleen
> 
> Thanks,
> Serguei
> 
> 
> On 3/27/19 06:09, coleen.phillimore@oracle.com wrote:
> > Summary: Note nmethods with "old" Methods in them in table to walk 
> > instead.
> > 
> > See RFE for more details.   Tested with RedefineClasses tests 
> > with/without -Xcomp, mach5 tier1-5.
> > 
> > open webrev at 
> > http://cr.openjdk.java.net/~coleenp/2019/8221183.01/webrev
> > bug link https://bugs.openjdk.java.net/browse/JDK-8221183
> > 
> > Thanks,
> > Coleen
> 


[Attachment #3 (text/html)]

<html>
  <head>
    <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <br>
    Serguei,<br>
    Thank you for reviewing.<br>
    <br>
    <div class="moz-cite-prefix">On 3/29/19 3:06 PM,
      <a class="moz-txt-link-abbreviated" \
href="mailto:serguei.spitsyn@oracle.com">serguei.spitsyn@oracle.com</a> wrote:<br>  \
</div>  <blockquote type="cite"
      cite="mid:8970aca2-faa7-268e-6043-a7a269af4706@oracle.com">
      <meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
      <div class="moz-cite-prefix">Hi Coleen,<br>
        <br>
        It looks good to me.<br>
        I agree with Erik on reset_old_table().<br>
        <br>
        I have one question.<br>
        <br>
        <a class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/~coleenp/2019/8221183.01/webrev/src/hotspot/share/code/codeCache.cpp.frames.html"
                
          moz-do-not-send="true">http://cr.openjdk.java.net/~coleenp/2019/8221183.01/webrev/src/hotspot/share/code/codeCache.cpp.frames.html</a><br>
  <br>
      </div>
    </blockquote>
    <br>
    Yes, I changed it to reset_old_method_table.<br>
    <br>
    <blockquote type="cite"
      cite="mid:8970aca2-faa7-268e-6043-a7a269af4706@oracle.com">
      <div class="moz-cite-prefix">
        <pre><span class="new">1077 void \
CodeCache::mark_for_evol_deoptimization(InstanceKlass* dependee) {</span> <b><span \
class="new">1078   MutexLockerEx mu(CodeCache_lock, \
Mutex::_no_safepoint_check_flag);</span></b> 1079 
1080   // Mark dependent AOT nmethods, which are only found via the class redefined.
<span class="new">1081   // TODO: add dependencies to aotCompiledMethod's metadata \
section so this isn't</span> <span class="new">1082   // needed.</span>
1083   AOTLoader::mark_evol_dependent_methods(dependee);
1084 }</pre>
        <br>
        Is it still necessary to grab the <span class="new">CodeCache_lock
          here?<br>
          <br>
        </span></div>
    </blockquote>
    It's actually not necessary because we're at a safepoint, and this
    code is always at a safepoint.<br>
    <br>
    I'll assert it's at a safepoint.   flush_evol_dependents has this:<br>
    <br>
       // --- Compile_lock is not held. However we are at a safepoint.<br>
       assert_locked_or_safepoint(Compile_lock);<br>
    <br>
    Thanks,<br>
    Coleen<br>
    <blockquote type="cite"
      cite="mid:8970aca2-faa7-268e-6043-a7a269af4706@oracle.com">
      <div class="moz-cite-prefix"><span class="new"> <br>
          Thanks,<br>
          Serguei</span> <br>
        <br>
        <br>
        On 3/27/19 06:09, <a class="moz-txt-link-abbreviated"
          href="mailto:coleen.phillimore@oracle.com"
          moz-do-not-send="true">coleen.phillimore@oracle.com</a> wrote:<br>
      </div>
      <blockquote type="cite"
        cite="mid:a140ff5b-2fb2-07da-eaac-8e204c629efe@oracle.com">Summary:
        Note nmethods with "old" Methods in them in table to walk
        instead. <br>
        <br>
        See RFE for more details.   Tested with RedefineClasses tests
        with/without -Xcomp, mach5 tier1-5. <br>
        <br>
        open webrev at <a class="moz-txt-link-freetext"
          href="http://cr.openjdk.java.net/~coleenp/2019/8221183.01/webrev"
          moz-do-not-send="true">http://cr.openjdk.java.net/~coleenp/2019/8221183.01/webrev</a>
  <br>
        bug link <a class="moz-txt-link-freetext"
          href="https://bugs.openjdk.java.net/browse/JDK-8221183"
          moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8221183</a>
        <br>
        <br>
        Thanks, <br>
        Coleen <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