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
--- 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; }