<html>
  <head>
    <meta content="text/html; charset=utf-8" http-equiv="Content-Type">
  </head>
  <body bgcolor="#FFFFFF" text="#000000">
    <pre><span class="new">Hi Albert,

 256       {</span>
<span class="new"> 257         ThreadBlockInVM tbivm(JavaThread::current());</span>
<span class="new"> 258         MutexLockerEx waiter(CodeCache_lock, Mutex::_no_safepoint_check_flag);</span>
<span class="new"> 259         const long wait_time = 60*60*24 * 1000;</span>
<span class="new"> 260         timeout = CodeCache_lock->wait(Mutex::_no_safepoint_check_flag, wait_time);</span>
<span class="new"> 261       }</span>
<span class="new"> 262       // We need to check for the safepoint to be able to exit the VM</span>
<span class="new"> 263       MutexLockerEx waiter(CodeCache_lock, Mutex::_no_safepoint_check_flag);</span>
<span class="new"> 264       NMethodSweeper::handle_safepoint_request();</span></pre>
    How about simplifying the above so that you don't release and then
    reacquire the lock:<br>
    <br>
    <pre><span class="new"> 256       {</span>
<span class="new"> 257         ThreadBlockInVM tbivm(JavaThread::current());</span>
<span class="new"> 258         MutexLockerEx waiter(CodeCache_lock, Mutex::_no_safepoint_check_flag);</span>
<span class="new"> 259         const long wait_time = 60*60*24 * 1000;</span>
<span class="new"> 260         timeout = CodeCache_lock->wait(Mutex::_no_safepoint_check_flag, wait_time);</span>
<span class="new"> 261         // We need to check for the safepoint to be able to exit the VM</span>
<span class="new"> 262         NMethodSweeper::handle_safepoint_request();</span>
 <span class="new">263       }</span>
</pre>
    <br>
    (This is not a full review, just a comment.)<br>
    <br>
    dl<br>
    <br>
    <div class="moz-cite-prefix">On 10/9/2014 1:24 AM, Albert Noll
      wrote:<br>
    </div>
    <blockquote cite="mid:5436465A.3080001@oracle.com" type="cite">
      <meta http-equiv="content-type" content="text/html; charset=utf-8">
      Hi,<br>
      <br>
      could I get reviews for this patch?<br>
      <br>
      Bug:<br>
      <a moz-do-not-send="true" class="moz-txt-link-freetext"
        href="https://bugs.openjdk.java.net/browse/JDK-8046809">https://bugs.openjdk.java.net/browse/JDK-8046809</a><br>
      <br>
      Problem:<br>
      <meta http-equiv="content-type" content="text/html; charset=utf-8">
      The test generates an artificially large number of adapters.
      CodeCacheMinimumFreeSpace in not large enough to hold all
      adapters, <br>
      if the code cache gets full. Furthermore, the JVM is in a state
      where no safepoint is requested. As a result, stack scanning of
      active <br>
      methods does not happen and consequently nmethods cannot be
      flushed from the code cache.<br>
      <br>
      Solution:<br>
      The following two changes fix the problem:<br>
      <br>
      <meta http-equiv="content-type" content="text/html; charset=utf-8">
      1) Introduce a new VM operation that forces stack scanning of
      active methods<br>
      <br>
      In the current design, the code cache sweeper can only make
      progress (i.e., remove compiled code) if<br>
      safepoints are triggered. More specifically, several safepoints
      must occur to concert compiled code from<br>
      state 'not_entrant' to 'unloaded'. If no safepoints are triggered,
      code cannot be removed from the code<br>
      cache. If the code cache fills up and safepoints are triggered too
      infrequently, the sweeper cannot remove <br>
      compiled code from the code cache fast enough. <br>
      <br>
      This patch forces a VM operation to do stack scanning, if there is
      10% free space in the code cache. Is parameter <br>
      is currently constant. I command line parameter can be added to
      provide the user with explicit control over this<br>
      threshold.  I think we can add such a command line parameter on
      demand.<br>
      <br>
      2) Use a dedicated sweeper thread that processes the whole code
      cache at once (remove NmethodSweepFraction)<br>
      Using a separate sweeper thread comes at the cost requiring more
      memory (mostly the stack size of the sweeper <br>
      thread) but has numerous benefits:<br>
      a) Code gets simpler: We can remove NMethodSweepFraction and 
      NmethodSweepCheckInterval<br>
      b) More aggressive sweeping: If the code cache gets full, we can
      process the entire code<br>
      cache without taking away cycles that are potentially needed for
      compilation.<br>
      <br>
      The patch also removes CodeCacheMinimumFreeSpace and 'critical'
      code cache allocations. Due to a bug in<br>
      heap.cpp, the CodeCacheMinimumFreeSpace was in fact not reserved
      for 'critical' allocations. The following <br>
      lines produce an underflow if heap_unallocated_capacity() <
      CodeCacheMinimumFreeSpace:<br>
      <br>
      segments_to_size(number_of_segments) >
      (heap_unallocated_capacity() - CodeCacheMinimumFreeSpace)<br>
      <br>
      Since the critical part in the code cache was never used for its
      intended purpose and we did not have problems<br>
      due to that, we can remove it.<br>
      <br>
      Correctness testing:<br>
      Failing test case, JPRT<br>
      <br>
      Performance testing:<br>
      The bug contains a link to performance results.<br>
      <br>
      Webrev:<br>
      <a moz-do-not-send="true" class="moz-txt-link-freetext"
        href="http://cr.openjdk.java.net/%7Eanoll/8046809/webrev.02/">http://cr.openjdk.java.net/~anoll/8046809/webrev.02/</a><br>
      <br>
      Many thanks in advance,<br>
      Albert<br>
    </blockquote>
    <br>
  </body>
</html>