8191052: [Graal] java/lang/invoke/CallSiteTest.java intermittently fails with "Failed dependency of type call_site_target_value" when running with Graal as JIT
authornever
Mon, 04 Dec 2017 13:13:44 -0800
changeset 48299 e8f5fc8f5f67
parent 48298 40b9faefb496
child 48300 8a5edac3d5a2
8191052: [Graal] java/lang/invoke/CallSiteTest.java intermittently fails with "Failed dependency of type call_site_target_value" when running with Graal as JIT Reviewed-by: kvn, iveresov, dlong
src/hotspot/share/ci/ciEnv.cpp
src/hotspot/share/code/dependencies.cpp
src/hotspot/share/code/dependencies.hpp
src/hotspot/share/jvmci/jvmciEnv.cpp
src/hotspot/share/jvmci/jvmciEnv.hpp
--- a/src/hotspot/share/ci/ciEnv.cpp	Sat Dec 02 13:50:04 2017 +0100
+++ b/src/hotspot/share/ci/ciEnv.cpp	Mon Dec 04 13:13:44 2017 -0800
@@ -899,64 +899,18 @@
 void ciEnv::validate_compile_task_dependencies(ciMethod* target) {
   if (failing())  return;  // no need for further checks
 
-  // First, check non-klass dependencies as we might return early and
-  // not check klass dependencies if the system dictionary
-  // modification counter hasn't changed (see below).
-  for (Dependencies::DepStream deps(dependencies()); deps.next(); ) {
-    if (deps.is_klass_type())  continue;  // skip klass dependencies
-    Klass* witness = deps.check_dependency();
-    if (witness != NULL) {
-      if (deps.type() == Dependencies::call_site_target_value) {
-        _inc_decompile_count_on_failure = false;
-        record_failure("call site target change");
-      } else {
-        record_failure("invalid non-klass dependency");
-      }
-      return;
+  bool counter_changed = system_dictionary_modification_counter_changed();
+  Dependencies::DepType result = dependencies()->validate_dependencies(_task, counter_changed);
+  if (result != Dependencies::end_marker) {
+    if (result == Dependencies::call_site_target_value) {
+      _inc_decompile_count_on_failure = false;
+      record_failure("call site target change");
+    } else if (Dependencies::is_klass_type(result)) {
+      record_failure("invalid non-klass dependency");
+    } else {
+      record_failure("concurrent class loading");
     }
   }
-
-  // Klass dependencies must be checked when the system dictionary
-  // changes.  If logging is enabled all violated dependences will be
-  // recorded in the log.  In debug mode check dependencies even if
-  // the system dictionary hasn't changed to verify that no invalid
-  // dependencies were inserted.  Any violated dependences in this
-  // case are dumped to the tty.
-  bool counter_changed = system_dictionary_modification_counter_changed();
-
-  bool verify_deps = trueInDebug;
-  if (!counter_changed && !verify_deps)  return;
-
-  int klass_violations = 0;
-  for (Dependencies::DepStream deps(dependencies()); deps.next(); ) {
-    if (!deps.is_klass_type())  continue;  // skip non-klass dependencies
-    Klass* witness = deps.check_dependency();
-    if (witness != NULL) {
-      klass_violations++;
-      if (!counter_changed) {
-        // Dependence failed but counter didn't change.  Log a message
-        // describing what failed and allow the assert at the end to
-        // trigger.
-        deps.print_dependency(witness);
-      } else if (xtty == NULL) {
-        // If we're not logging then a single violation is sufficient,
-        // otherwise we want to log all the dependences which were
-        // violated.
-        break;
-      }
-    }
-  }
-
-  if (klass_violations != 0) {
-#ifdef ASSERT
-    if (!counter_changed && !PrintCompilation) {
-      // Print out the compile task that failed
-      _task->print_tty();
-    }
-#endif
-    assert(counter_changed, "failed dependencies, but counter didn't change");
-    record_failure("concurrent class loading");
-  }
 }
 
 // ------------------------------------------------------------------
--- a/src/hotspot/share/code/dependencies.cpp	Sat Dec 02 13:50:04 2017 +0100
+++ b/src/hotspot/share/code/dependencies.cpp	Mon Dec 04 13:13:44 2017 -0800
@@ -30,6 +30,8 @@
 #include "classfile/javaClasses.inline.hpp"
 #include "code/dependencies.hpp"
 #include "compiler/compileLog.hpp"
+#include "compiler/compileBroker.hpp"
+#include "compiler/compileTask.hpp"
 #include "memory/resourceArea.hpp"
 #include "oops/oop.inline.hpp"
 #include "oops/objArrayKlass.hpp"
@@ -620,6 +622,72 @@
   guarantee(FIRST_TYPE <= dept && dept < TYPE_LIMIT, "invalid dependency type: %d", (int) dept);
 }
 
+Dependencies::DepType Dependencies::validate_dependencies(CompileTask* task, bool counter_changed, char** failure_detail) {
+  // First, check non-klass dependencies as we might return early and
+  // not check klass dependencies if the system dictionary
+  // modification counter hasn't changed (see below).
+  for (Dependencies::DepStream deps(this); deps.next(); ) {
+    if (deps.is_klass_type())  continue;  // skip klass dependencies
+    Klass* witness = deps.check_dependency();
+    if (witness != NULL) {
+      return deps.type();
+    }
+  }
+
+  // Klass dependencies must be checked when the system dictionary
+  // changes.  If logging is enabled all violated dependences will be
+  // recorded in the log.  In debug mode check dependencies even if
+  // the system dictionary hasn't changed to verify that no invalid
+  // dependencies were inserted.  Any violated dependences in this
+  // case are dumped to the tty.
+  if (!counter_changed && !trueInDebug) {
+    return end_marker;
+  }
+
+  int klass_violations = 0;
+  DepType result = end_marker;
+  for (Dependencies::DepStream deps(this); deps.next(); ) {
+    if (!deps.is_klass_type())  continue;  // skip non-klass dependencies
+    Klass* witness = deps.check_dependency();
+    if (witness != NULL) {
+      if (klass_violations == 0) {
+        result = deps.type();
+        if (failure_detail != NULL && klass_violations == 0) {
+          // Use a fixed size buffer to prevent the string stream from
+          // resizing in the context of an inner resource mark.
+          char* buffer = NEW_RESOURCE_ARRAY(char, O_BUFLEN);
+          stringStream st(buffer, O_BUFLEN);
+          deps.print_dependency(witness, true, &st);
+          *failure_detail = st.as_string();
+        }
+      }
+      klass_violations++;
+      if (!counter_changed) {
+        // Dependence failed but counter didn't change.  Log a message
+        // describing what failed and allow the assert at the end to
+        // trigger.
+        deps.print_dependency(witness);
+      } else if (xtty == NULL) {
+        // If we're not logging then a single violation is sufficient,
+        // otherwise we want to log all the dependences which were
+        // violated.
+        break;
+      }
+    }
+  }
+
+  if (klass_violations != 0) {
+#ifdef ASSERT
+    if (task != NULL && !counter_changed && !PrintCompilation) {
+      // Print out the compile task that failed
+      task->print_tty();
+    }
+#endif
+    assert(counter_changed, "failed dependencies, but counter didn't change");
+  }
+  return result;
+}
+
 // for the sake of the compiler log, print out current dependencies:
 void Dependencies::log_all_dependencies() {
   if (log() == NULL)  return;
--- a/src/hotspot/share/code/dependencies.hpp	Sat Dec 02 13:50:04 2017 +0100
+++ b/src/hotspot/share/code/dependencies.hpp	Mon Dec 04 13:13:44 2017 -0800
@@ -457,6 +457,8 @@
 
   void copy_to(nmethod* nm);
 
+  DepType validate_dependencies(CompileTask* task, bool counter_changed, char** failure_detail = NULL);
+
   void log_all_dependencies();
 
   void log_dependency(DepType dept, GrowableArray<ciBaseObject*>* args) {
--- a/src/hotspot/share/jvmci/jvmciEnv.cpp	Sat Dec 02 13:50:04 2017 +0100
+++ b/src/hotspot/share/jvmci/jvmciEnv.cpp	Mon Dec 04 13:13:44 2017 -0800
@@ -410,8 +410,8 @@
 // ------------------------------------------------------------------
 // Check for changes to the system dictionary during compilation
 // class loads, evolution, breakpoints
-JVMCIEnv::CodeInstallResult JVMCIEnv::check_for_system_dictionary_modification(Dependencies* dependencies, Handle compiled_code,
-                                                                               JVMCIEnv* env, char** failure_detail) {
+JVMCIEnv::CodeInstallResult JVMCIEnv::validate_compile_task_dependencies(Dependencies* dependencies, Handle compiled_code,
+                                                                         JVMCIEnv* env, char** failure_detail) {
   // If JVMTI capabilities were enabled during compile, the compilation is invalidated.
   if (env != NULL) {
     if (!env->_jvmti_can_hotswap_or_post_breakpoint && JvmtiExport::can_hotswap_or_post_breakpoint()) {
@@ -422,37 +422,20 @@
 
   // Dependencies must be checked when the system dictionary changes
   // or if we don't know whether it has changed (i.e., env == NULL).
-  // In debug mode, always check dependencies.
-  bool counter_changed = env != NULL && env->_system_dictionary_modification_counter != SystemDictionary::number_of_modifications();
-  bool verify_deps = env == NULL || trueInDebug || JavaAssertions::enabled(SystemDictionary::HotSpotInstalledCode_klass()->name()->as_C_string(), true);
-  if (!counter_changed && !verify_deps) {
+  bool counter_changed = env == NULL || env->_system_dictionary_modification_counter != SystemDictionary::number_of_modifications();
+  CompileTask* task = env == NULL ? NULL : env->task();
+  Dependencies::DepType result = dependencies->validate_dependencies(task, counter_changed, failure_detail);
+  if (result == Dependencies::end_marker) {
     return JVMCIEnv::ok;
   }
 
-  for (Dependencies::DepStream deps(dependencies); deps.next(); ) {
-    Klass* witness = deps.check_dependency();
-    if (witness != NULL) {
-      // Use a fixed size buffer to prevent the string stream from
-      // resizing in the context of an inner resource mark.
-      char* buffer = NEW_RESOURCE_ARRAY(char, O_BUFLEN);
-      stringStream st(buffer, O_BUFLEN);
-      deps.print_dependency(witness, true, &st);
-      *failure_detail = st.as_string();
-      if (env == NULL || counter_changed || deps.type() == Dependencies::evol_method) {
-        return JVMCIEnv::dependencies_failed;
-      } else {
-        // The dependencies were invalid at the time of installation
-        // without any intervening modification of the system
-        // dictionary.  That means they were invalidly constructed.
-        return JVMCIEnv::dependencies_invalid;
-      }
-    }
-    if (LogCompilation) {
-      deps.log_dependency();
-    }
+  if (!Dependencies::is_klass_type(result) || counter_changed) {
+    return JVMCIEnv::dependencies_failed;
   }
-
-  return JVMCIEnv::ok;
+  // The dependencies were invalid at the time of installation
+  // without any intervening modification of the system
+  // dictionary.  That means they were invalidly constructed.
+  return JVMCIEnv::dependencies_invalid;
 }
 
 // ------------------------------------------------------------------
@@ -492,8 +475,15 @@
     // Encode the dependencies now, so we can check them right away.
     dependencies->encode_content_bytes();
 
+    // Record the dependencies for the current compile in the log
+    if (LogCompilation) {
+      for (Dependencies::DepStream deps(dependencies); deps.next(); ) {
+        deps.log_dependency();
+      }
+    }
+
     // Check for {class loads, evolution, breakpoints} during compilation
-    result = check_for_system_dictionary_modification(dependencies, compiled_code, env, &failure_detail);
+    result = validate_compile_task_dependencies(dependencies, compiled_code, env, &failure_detail);
     if (result != JVMCIEnv::ok) {
       // While not a true deoptimization, it is a preemptive decompile.
       MethodData* mdp = method()->method_data();
--- a/src/hotspot/share/jvmci/jvmciEnv.hpp	Sat Dec 02 13:50:04 2017 +0100
+++ b/src/hotspot/share/jvmci/jvmciEnv.hpp	Mon Dec 04 13:13:44 2017 -0800
@@ -138,8 +138,8 @@
 
   // Helper routine for determining the validity of a compilation
   // with respect to concurrent class loading.
-  static JVMCIEnv::CodeInstallResult check_for_system_dictionary_modification(Dependencies* target, Handle compiled_code,
-                                                                              JVMCIEnv* env, char** failure_detail);
+  static JVMCIEnv::CodeInstallResult validate_compile_task_dependencies(Dependencies* target, Handle compiled_code,
+                                                                        JVMCIEnv* env, char** failure_detail);
 
 public:
   CompileTask* task() { return _task; }