<html><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;">What I mean is a strategy like this:<div><br><div><font face="Monaco" size="1">diff --git a/src/share/vm/compiler/compileBroker.cpp b/src/share/vm/compiler/compileBroker.cpp<br>--- a/src/share/vm/compiler/compileBroker.cpp<br>+++ b/src/share/vm/compiler/compileBroker.cpp<br>@@ -247,6 +247,7 @@<br><br>   _is_complete = false;<br>   _is_success = false;<br>+  _skip_compile = false;<br>   _code_handle = NULL;<br><br>   _hot_method = NULL;<br>@@ -1691,9 +1692,14 @@<br>     // Assign the task to the current thread.  Mark this compilation<br>     // thread as active for the profiler.<br>     CompileTaskWrapper ctw(task);<br>+    methodHandle method(thread, task->method());<br>+    if (task->skip_compile()) {<br>+      method->clear_queued_for_compilation();<br>+      continue;<br>+    }<br>+<br>     nmethodLocker result_handle;  // (handle for the nmethod produced by this task)<br>     task->set_code_handle(&result_handle);<br>-    methodHandle method(thread, task->method());<br><br>     // Never compile a method if breakpoints are present in it<br>     if (method()->number_of_breakpoints() == 0) {<br>diff --git a/src/share/vm/compiler/compileBroker.hpp b/src/share/vm/compiler/compileBroker.hpp<br>--- a/src/share/vm/compiler/compileBroker.hpp<br>+++ b/src/share/vm/compiler/compileBroker.hpp<br>@@ -48,6 +48,7 @@<br>   bool         _is_complete;<br>   bool         _is_success;<br>   bool         _is_blocking;<br>+  bool         _skip_compile;<br>   int          _comp_level;<br>   int          _num_inlined_bytecodes;<br>   nmethodLocker* _code_handle;  // holder of eventual result<br>@@ -78,6 +79,8 @@<br>   bool         is_blocking() const               { return _is_blocking; }<br>   bool         is_success() const                { return _is_success; }<br><br>+  bool         skip_compile() const              { return _skip_compile; }<br>+  void         set_skip_compile()                { _skip_compile = true; }<br>   nmethodLocker* code_handle() const             { return _code_handle; }<br>   void         set_code_handle(nmethodLocker* l) { _code_handle = l; }<br>   nmethod*     code() const;                     // _code_handle->code()<br>diff --git a/src/share/vm/runtime/advancedThresholdPolicy.cpp b/src/share/vm/runtime/advancedThresholdPolicy.cpp<br>--- a/src/share/vm/runtime/advancedThresholdPolicy.cpp<br>+++ b/src/share/vm/runtime/advancedThresholdPolicy.cpp<br>@@ -174,11 +174,8 @@<br>         if (PrintTieredEvents) {<br>           print_event(REMOVE_FROM_QUEUE, method, method, task->osr_bci(), (CompLevel)task->comp_level());<br>         }<br>-        CompileTaskWrapper ctw(task); // Frees the task<br>-        compile_queue->remove(task);<br>-        method->clear_queued_for_compilation();<br>-        task = next_task;<br>-        continue;<br>+        task->set_skip_compile();<br>+        return task;<br>       }<br><br>       // Select a method with a higher rate</font><br>On Mar 11, 2014, at 5:30 PM, Igor Veresov <<a href="mailto:igor.veresov@oracle.com">igor.veresov@oracle.com</a>> wrote:<br><br><blockquote type="cite">Yes, I think youíre right, this is a bug. select_task() obviously assumes there can be no safepoints. May be we can introduce a flag in CompileTask that would indicate that it needs to be removed, and then return it from select_task() and correspondingly from CompileQueue::get() and then free it with the existing CompileTaskWrapper that is CompileBroker::compiler_thread_loop() instead of invoking a compile. That way, combined with your existing fixes select_task() will be lock-free. Does that make sense?<br><br>igor<br><br>On Mar 11, 2014, at 4:34 PM, Vladimir Ivanov <<a href="mailto:vladimir.x.ivanov@oracle.com">vladimir.x.ivanov@oracle.com</a>> wrote:<br><br><blockquote type="cite">Igor,<br><br><blockquote type="cite">I vaguely remember that is was allowed before. Thatís basically the reason why everything has handles in the policy. I need to recall how that works...<br></blockquote>It's there for a long time, but I converted the check from VM warning to fatal error only recently.<br><br>AdvancedThresholdPolicy::select_task operates on raw Method*. As I can see in the sources, handles are used only in Method::build_method_counters. Lazy allocation of method counters wasn't there originally. It was added by 8010862.<br><br><blockquote type="cite">Btw, I may be wrong but it seems like there could be a race in MethodCounters creation. There is a similar problem with MDO, but we take a lock for it to avoid races.<br></blockquote>You are right. There's a window in Method::build_method_counters when counters can be allocated twice. We need to grab a lock / use CAS to avoid memory leak here.<br><br>Best regards,<br>Vladimir Ivanov<br><br>[1] <a href="https://bugs.openjdk.java.net/browse/JDK-8010862">https://bugs.openjdk.java.net/browse/JDK-8010862</a><br><br><blockquote type="cite"><br>igor<br><br>On Mar 11, 2014, at 3:04 PM, Vladimir Ivanov <<a href="mailto:vladimir.x.ivanov@oracle.com">vladimir.x.ivanov@oracle.com</a>> wrote:<br><br><blockquote type="cite">The policy for a thread is not to hold any locks VM can block on when entering a safepoint (see Thread::check_for_valid_safepoint_state).<br><br>Otherwise we would need to be very careful about what code can be executed during a safepoint to avoid deadlocks.<br><br>There are exceptions (like Threads_lock and Compile_lock), but generally we try to adhere the rule.<br><br>Making an exception for MethodCompileQueue looks safe (I went through the code and didn't find any scenarios when VM can attempt to grab it during a safepoint), but I'd like to avoid it if possible.<br><br>Best regards,<br>Vladimir Ivanov<br><br>On 3/11/14 10:50 PM, Igor Veresov wrote:<br><blockquote type="cite">Could you please remind me why we canít enter a safepoint while holding the MethodCompileQueue_lock?<br><br>igor<br><br>On Mar 11, 2014, at 8:50 AM, Vladimir Ivanov <<a href="mailto:vladimir.x.ivanov@oracle.com">vladimir.x.ivanov@oracle.com</a>> wrote:<br><br><blockquote type="cite">Unfortunately, it's not enough. There's another safepoint check.<br><br>For blocking compilation requests of stale methods CompileTaskWrapper (see AdvancedThresholdPolicy::select_task) sends a notification to blocked threads after cancelling the compilation. It can safepoint while locking on compile task before sending notification.<br><br>I don't see how to avoid this situation. Any ideas?<br>Otherwise, I need to exclude MethodCompileQueue from the check in Thread::check_for_valid_safepoint_state.<br><br>Best regards,<br>Vladimir Ivanov<br><br>On 3/11/14 11:58 AM, Vladimir Ivanov wrote:<br><blockquote type="cite">Igor, Vladimir, thanks for review.<br><br>Best regards,<br>Vladimir Ivanov<br><br>On 3/11/14 7:31 AM, Igor Veresov wrote:<br><blockquote type="cite">I think itís a reasonable fix.<br><br>igor<br><br>On Mar 10, 2014, at 4:57 PM, Vladimir Ivanov<br><<a href="mailto:vladimir.x.ivanov@oracle.com">vladimir.x.ivanov@oracle.com</a>> wrote:<br><br><blockquote type="cite">Vladimir, thanks for the review.<br><br>You are absolutely right about<br>Method::increment_interpreter_invocation_count. Reverted the change.<br><br>Updated fix:<br><a href="http://cr.openjdk.java.net/~vlivanov/8023461/webrev.01/">http://cr.openjdk.java.net/~vlivanov/8023461/webrev.01/</a><br><br>Yes, Igor's feedback on this change would be invaluable.<br><br>Best regards,<br>Vladimir Ivanov<br><br>On 3/11/14 2:33 AM, Vladimir Kozlov wrote:<br><blockquote type="cite">The method Method::increment_interpreter_invocation_count(TRAP) changes<br>are incorrect. It is used by C++ Interpreter and you did not modified<br>code there. I would leave this method unchanged.<br><br>The rest looks fine to me but Igor should know better this code.<br><br>Thanks,<br>Vladimir K<br><br>On 3/7/14 8:26 AM, Vladimir Ivanov wrote:<br><blockquote type="cite">http://cr.openjdk.java.net/~vlivanov/8023461/webrev.00<br>https://bugs.openjdk.java.net/browse/JDK-8023461<br>42 lines changed: 13 ins; 1 del; 28 mod<br><br>The rule of thumb for VM is that a thread shouldn't hold any VM lock<br>when it reaches a safepoint. It's not the case for<br>MethodCompileQueue_lock now.<br><br>The problem is that AdvancedThresholdPolicy updates task's rate when<br>iterating compiler queue. It holds MethodCompileQueue_lock while doing<br>so. Method counters are allocated lazily. If method counters aren't<br>there and VM fails to allocate them, GC is initiated (see<br>CollectorPolicy::satisfy_failed_metadata_allocation) and a thead<br>entering a safepoint holding MethodCompileQueue lock.<br><br>Normally, counters are initialized during method interpretation,<br>but in<br>Xcomp mode it's not the case. That's the mode where the failures are<br>observed.<br><br>The fix is to skip the update, if counters aren't allocated yet.<br><br>Testing: added No_Safepoint_Verifier, JPRT, failing tests from nightly<br>testing (in progress).<br><br>Best regards,<br>Vladimir Ivanov<br></blockquote></blockquote></blockquote><br></blockquote></blockquote></blockquote><br></blockquote></blockquote><br></blockquote></blockquote><br></blockquote><br></div></div></body></html>