<html>
  <head>
    <meta content="text/html; charset=ISO-8859-1"
      http-equiv="Content-Type">
  </head>
  <body text="#000000" bgcolor="#FFFFFF">
    <div class="moz-cite-prefix">Hi Vladimir,<br>
      <br>
      thanks for looking at the patch. <br>
      Here is the new webrev:<br>
      <meta http-equiv="content-type" content="text/html;
        charset=ISO-8859-1">
      <a href="http://cr.openjdk.java.net/%7Eanoll/8023014/webrev.01/">http://cr.openjdk.java.net/~anoll/8023014/webrev.01/</a><br>
      <br>
      See comments inline:<br>
      <br>
      On 26.09.2013 00:48, Vladimir Kozlov wrote:<br>
    </div>
    <blockquote cite="mid:52436843.6010008@oracle.com" type="cite">Thank
      you, Albert
      <br>
      <br>
      I would suggest to do TieredStartAtLevel changes separately, it is
      not related to this bug.
      <br>
      <br>
    </blockquote>
    Done.<br>
    <blockquote cite="mid:52436843.6010008@oracle.com" type="cite">Second,
      how we can guard the initialization with a field (_compiler_state
      and original _is_initialized) which is not static?
      <br>
      <br>
      You don't need separate local do_init in C*_Compiler.cpp since it
      is used only once:
      <br>
      <br>
    </blockquote>
    Done.<br>
    <blockquote cite="mid:52436843.6010008@oracle.com" type="cite">+&nbsp; if
      (should_perform_init()) {
      <br>
      <br>
      abstractCompiler.cpp:
      <br>
      You removed ThreadInVMfromNative and ResetNoHandleMark from
      should_perform_init() and set_state(). Why? There is assert in
      check_prelock_state() called from Monitor::lock():
      <br>
      <br>
      &nbsp; assert((!thread-&gt;is_Java_thread() || ((JavaThread
      *)thread)-&gt;thread_state() == _thread_in_vm)
      <br>
      &nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; || rank() == Mutex::special, "wrong thread state for
      using locks");
      <br>
      <br>
      Compiler threads are Java threads, so you should be in VM state
      when lock. How you did not hit this assert?
      <br>
      <br>
    </blockquote>
    The state transition from native to VM (and ResetNoHandleMark) is
    now in compileBroker.cpp:1543.<br>
    <blockquote cite="mid:52436843.6010008@oracle.com" type="cite">
      <br>
      Typo in abstractCompiler.hpp 'methos':
      <br>
      <br>
      +&nbsp; // This method returns true for the first compiler thread that
      reaches that methos.
      <br>
      <br>
      It is not 'compiler object', it is 'compiler runtime' (yes it was
      before but it was not accurate, it could be mistaking for Compile
      object created for each compilation):
      <br>
      <br>
      +&nbsp; // This thread will initialize the compiler object.
      <br>
      <br>
      Swap lines (and use 'runtime' in the comment):
      <br>
      <br>
      !&nbsp;&nbsp; bool is_initialized&nbsp;&nbsp;&nbsp;&nbsp; ()&nbsp;&nbsp;&nbsp;&nbsp;&nbsp;&nbsp; { return _compiler_state ==
      initialized; }
      <br>
      !&nbsp;&nbsp; // Get/set state of compiler objects
      <br>
      <br>
      <br>
      c2compiler.cpp:
      <br>
      <br>
      C2Compiler:: is not needed:
      <br>
      <br>
      +&nbsp;&nbsp;&nbsp; bool successful = C2Compiler::init_c2_runtime();
      <br>
      <br>
      <br>
    </blockquote>
    Fixed the issues above.<br>
    <blockquote cite="mid:52436843.6010008@oracle.com" type="cite">Should
      we exit Compiler thread which failed local buffer allocation
      instead parking? Why keep resources allocated for it?
      <br>
      <br>
    </blockquote>
    You are right, there is no reason the keep these threads around.
    Compiler threads wo are not initialized correctly<br>
    now exit the VM.<br>
    <br>
    The patch is currently in west queue.<br>
    <br>
    Many thanks again,<br>
    Albert<br>
    <br>
    <blockquote cite="mid:52436843.6010008@oracle.com" type="cite">Thanks,
      <br>
      Vladimir
      <br>
      <br>
      On 9/25/13 1:05 PM, Albert Noll wrote:
      <br>
      <blockquote type="cite">Hi,
        <br>
        <br>
        could I have a review for this patch?
        <br>
        <br>
        bug: <a class="moz-txt-link-freetext" href="https://bugs.openjdk.java.net/browse/JDK-8023014">https://bugs.openjdk.java.net/browse/JDK-8023014</a>
        <br>
        webrev: <a class="moz-txt-link-freetext" href="http://cr.openjdk.java.net/~anoll/8023014/webrev.00/">http://cr.openjdk.java.net/~anoll/8023014/webrev.00/</a>
        <br>
        <a class="moz-txt-link-rfc2396E" href="http://cr.openjdk.java.net/%7Eanoll/8023014/webrev.00/">&lt;http://cr.openjdk.java.net/%7Eanoll/8023014/webrev.00/&gt;</a>
        <br>
        <br>
        Problem: C1/C2 compiler initialization can fail if there is not
        enough
        <br>
        memory in the code cache to generate all stubs.
        <br>
        This problem occurs only with a small code cache size.
        <br>
        <br>
        Solution: Check correctness of (parts of) C1/C2 initialization
        and
        <br>
        disable compiler (C1 and/or C2) if the compiler / thread
        <br>
        was not initialized correctly.
        <br>
        <br>
        Testing: jprt, test case, benchmarks
        <br>
        <br>
        This patch also contains a refactored version of compiler
        object/thread
        <br>
        initialization. In the new version, a compiler object/thread is
        <br>
        initialized prior entering the main compiler loop. In the old
        version,
        <br>
        the check if the compiler/thread is initialized was
        <br>
        done in the main compiler loop (and hence before every
        compilation).
        <br>
        <br>
        To continue execution with a limited code cache size, the code
        types
        <br>
        'AdapterBlob' and 'MethodHandlesAdapterBlob' must
        <br>
        be 'critical' allocations in the code cache, i.e., they must use
        space
        <br>
        reserved by CodeCacheMinimumFreeSpace.
        <br>
        <br>
        In addition I removed some unued code.
        <br>
        <br>
        Many thanks in advance for reviewing.
        <br>
        <br>
        Best,
        <br>
        Albert
        <br>
      </blockquote>
    </blockquote>
    <br>
  </body>
</html>