7083184: JSR 292: don't store context class argument with call site dependencies

Reviewed-by: jrose, never

hotspot/src/share/vm/ci/ciEnv.cpp

@@ -884,19 +884,31 @@ bool ciEnv::system_dictionary_modification_counter_changed() {
 }
 
 // ------------------------------------------------------------------
-// ciEnv::check_for_system_dictionary_modification
+// ciEnv::validate_compile_task_dependencies
-// Check for changes to the system dictionary during compilation
+//
-// class loads, evolution, breakpoints
+// Check for changes during compilation (e.g. class loads, evolution,
-void ciEnv::check_for_system_dictionary_modification(ciMethod* target) {
+// breakpoints, call site invalidation).
+void ciEnv::validate_compile_task_dependencies(ciMethod* target) {
   if (failing())  return;  // no need for further checks
 
-  // Dependencies must be checked when the system dictionary changes.
+  // First, check non-klass dependencies as we might return early and
-  // If logging is enabled all violated dependences will be recorded in
+  // not check klass dependencies if the system dictionary
-  // the log.  In debug mode check dependencies even if the system
+  // modification counter hasn't changed (see below).
-  // dictionary hasn't changed to verify that no invalid dependencies
+  for (Dependencies::DepStream deps(dependencies()); deps.next(); ) {
-  // were inserted.  Any violated dependences in this case are dumped to
+    if (deps.is_klass_type())  continue;  // skip klass dependencies
-  // the tty.
+    klassOop witness = deps.check_dependency();
+    if (witness != NULL) {
+      record_failure("invalid non-klass dependency");
+      return;
+    }
+  }
 
+  // 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 test_deps = counter_changed;
   DEBUG_ONLY(test_deps = true);
@@ -904,22 +916,21 @@ void ciEnv::check_for_system_dictionary_modification(ciMethod* target) {
 
   bool print_failures = false;
   DEBUG_ONLY(print_failures = !counter_changed);
-
   bool keep_going = (print_failures || xtty != NULL);
-
+  int klass_violations = 0;
-  int violated = 0;
 
   for (Dependencies::DepStream deps(dependencies()); deps.next(); ) {
+    if (!deps.is_klass_type())  continue;  // skip non-klass dependencies
     klassOop witness = deps.check_dependency();
     if (witness != NULL) {
-      ++violated;
+      klass_violations++;
       if (print_failures)  deps.print_dependency(witness, /*verbose=*/ true);
-      // If there's no log and we're not sanity-checking, we're done.
-      if (!keep_going)     break;
     }
+    // If there's no log and we're not sanity-checking, we're done.
+    if (!keep_going)  break;
   }
 
-  if (violated != 0) {
+  if (klass_violations != 0) {
     assert(counter_changed, "failed dependencies, but counter didn't change");
     record_failure("concurrent class loading");
   }
@@ -978,8 +989,8 @@ void ciEnv::register_method(ciMethod* target,
       // Encode the dependencies now, so we can check them right away.
       dependencies()->encode_content_bytes();
 
-      // Check for {class loads, evolution, breakpoints} during compilation
+      // Check for {class loads, evolution, breakpoints, ...} during compilation
-      check_for_system_dictionary_modification(target);
+      validate_compile_task_dependencies(target);
     }
 
     methodHandle method(THREAD, target->get_methodOop());

hotspot/src/share/vm/ci/ciEnv.hpp

@@ -247,9 +247,9 @@ private:
   // Is this thread currently in the VM state?
   static bool is_in_vm();
 
-  // Helper routine for determining the validity of a compilation
+  // Helper routine for determining the validity of a compilation with
-  // with respect to concurrent class loading.
+  // respect to method dependencies (e.g. concurrent class loading).
-  void check_for_system_dictionary_modification(ciMethod* target);
+  void validate_compile_task_dependencies(ciMethod* target);
 
 public:
   enum {

hotspot/src/share/vm/code/dependencies.cpp

@@ -113,9 +113,9 @@ void Dependencies::assert_has_no_finalizable_subclasses(ciKlass* ctxk) {
   assert_common_1(no_finalizable_subclasses, ctxk);
 }
 
-void Dependencies::assert_call_site_target_value(ciKlass* ctxk, ciCallSite* call_site, ciMethodHandle* method_handle) {
+void Dependencies::assert_call_site_target_value(ciCallSite* call_site, ciMethodHandle* method_handle) {
-  check_ctxk(ctxk);
+  check_ctxk(call_site->klass());
-  assert_common_3(call_site_target_value, ctxk, call_site, method_handle);
+  assert_common_2(call_site_target_value, call_site, method_handle);
 }
 
 // Helper function.  If we are adding a new dep. under ctxk2,
@@ -135,7 +135,7 @@ bool Dependencies::maybe_merge_ctxk(GrowableArray<ciObject*>* deps,
   }
 }
 
-void Dependencies::assert_common_1(Dependencies::DepType dept, ciObject* x) {
+void Dependencies::assert_common_1(DepType dept, ciObject* x) {
   assert(dep_args(dept) == 1, "sanity");
   log_dependency(dept, x);
   GrowableArray<ciObject*>* deps = _deps[dept];
@@ -148,21 +148,37 @@ void Dependencies::assert_common_1(Dependencies::DepType dept, ciObject* x) {
   }
 }
 
-void Dependencies::assert_common_2(Dependencies::DepType dept,
+void Dependencies::assert_common_2(DepType dept,
-                                   ciKlass* ctxk, ciObject* x) {
+                                   ciObject* x0, ciObject* x1) {
-  assert(dep_context_arg(dept) == 0, "sanity");
   assert(dep_args(dept) == 2, "sanity");
-  log_dependency(dept, ctxk, x);
+  log_dependency(dept, x0, x1);
   GrowableArray<ciObject*>* deps = _deps[dept];
 
   // see if the same (or a similar) dep is already recorded
-  if (note_dep_seen(dept, x)) {
+  bool has_ctxk = has_explicit_context_arg(dept);
-    // look in this bucket for redundant assertions
+  if (has_ctxk) {
-    const int stride = 2;
+    assert(dep_context_arg(dept) == 0, "sanity");
-    for (int i = deps->length(); (i -= stride) >= 0; ) {
+    if (note_dep_seen(dept, x1)) {
-      ciObject* x1 = deps->at(i+1);
+      // look in this bucket for redundant assertions
-      if (x == x1) {  // same subject; check the context
+      const int stride = 2;
-        if (maybe_merge_ctxk(deps, i+0, ctxk)) {
+      for (int i = deps->length(); (i -= stride) >= 0; ) {
+        ciObject* y1 = deps->at(i+1);
+        if (x1 == y1) {  // same subject; check the context
+          if (maybe_merge_ctxk(deps, i+0, x0->as_klass())) {
+            return;
+          }
+        }
+      }
+    }
+  } else {
+    assert(dep_implicit_context_arg(dept) == 0, "sanity");
+    if (note_dep_seen(dept, x0) && note_dep_seen(dept, x1)) {
+      // look in this bucket for redundant assertions
+      const int stride = 2;
+      for (int i = deps->length(); (i -= stride) >= 0; ) {
+        ciObject* y0 = deps->at(i+0);
+        ciObject* y1 = deps->at(i+1);
+        if (x0 == y0 && x1 == y1) {
           return;
         }
       }
@@ -170,11 +186,11 @@ void Dependencies::assert_common_2(Dependencies::DepType dept,
   }
 
   // append the assertion in the correct bucket:
-  deps->append(ctxk);
+  deps->append(x0);
-  deps->append(x);
+  deps->append(x1);
 }
 
-void Dependencies::assert_common_3(Dependencies::DepType dept,
+void Dependencies::assert_common_3(DepType dept,
                                    ciKlass* ctxk, ciObject* x, ciObject* x2) {
   assert(dep_context_arg(dept) == 0, "sanity");
   assert(dep_args(dept) == 3, "sanity");
@@ -361,7 +377,7 @@ int Dependencies::_dep_args[TYPE_LIMIT] = {
   3, // unique_concrete_subtypes_2 ctxk, k1, k2
   3, // unique_concrete_methods_2 ctxk, m1, m2
   1, // no_finalizable_subclasses ctxk
-  3  // call_site_target_value ctxk, call_site, method_handle
+  2  // call_site_target_value call_site, method_handle
 };
 
 const char* Dependencies::dep_name(Dependencies::DepType dept) {
@@ -375,10 +391,7 @@ int Dependencies::dep_args(Dependencies::DepType dept) {
 }
 
 void Dependencies::check_valid_dependency_type(DepType dept) {
-  for (int deptv = (int) FIRST_TYPE; deptv < (int) TYPE_LIMIT; deptv++) {
+  guarantee(FIRST_TYPE <= dept && dept < TYPE_LIMIT, err_msg("invalid dependency type: %d", (int) dept));
-    if (dept == ((DepType) deptv))  return;
-  }
-  ShouldNotReachHere();
 }
 
 // for the sake of the compiler log, print out current dependencies:
@@ -586,8 +599,7 @@ bool Dependencies::DepStream::next() {
     code_byte -= ctxk_bit;
     DepType dept = (DepType)code_byte;
     _type = dept;
-    guarantee((dept - FIRST_TYPE) < (TYPE_LIMIT - FIRST_TYPE),
+    Dependencies::check_valid_dependency_type(dept);
-              "bad dependency type tag");
     int stride = _dep_args[dept];
     assert(stride == dep_args(dept), "sanity");
     int skipj = -1;
@@ -615,18 +627,35 @@ oop Dependencies::DepStream::argument(int i) {
 
 klassOop Dependencies::DepStream::context_type() {
   assert(must_be_in_vm(), "raw oops here");
-  int ctxkj = dep_context_arg(_type);  // -1 if no context arg
+
-  if (ctxkj < 0) {
+  // Most dependencies have an explicit context type argument.
-    return NULL;           // for example, evol_method
+  {
-  } else {
+    int ctxkj = dep_context_arg(_type);  // -1 if no explicit context arg
-    oop k = recorded_oop_at(_xi[ctxkj]);
+    if (ctxkj >= 0) {
-    if (k != NULL) {       // context type was not compressed away
+      oop k = argument(ctxkj);
-      assert(k->is_klass(), "type check");
+      if (k != NULL) {       // context type was not compressed away
-      return (klassOop) k;
+        assert(k->is_klass(), "type check");
-    } else {               // recompute "default" context type
+        return (klassOop) k;
-      return ctxk_encoded_as_null(_type, recorded_oop_at(_xi[ctxkj+1]));
+      }
+      // recompute "default" context type
+      return ctxk_encoded_as_null(_type, argument(ctxkj+1));
     }
   }
+
+  // Some dependencies are using the klass of the first object
+  // argument as implicit context type (e.g. call_site_target_value).
+  {
+    int ctxkj = dep_implicit_context_arg(_type);
+    if (ctxkj >= 0) {
+      oop k = argument(ctxkj)->klass();
+      assert(k->is_klass(), "type check");
+      return (klassOop) k;
+    }
+  }
+
+  // And some dependencies don't have a context type at all,
+  // e.g. evol_method.
+  return NULL;
 }
 
 /// Checking dependencies:
@@ -1409,21 +1438,20 @@ klassOop Dependencies::check_has_no_finalizable_subclasses(klassOop ctxk, KlassD
 }
 
 
-klassOop Dependencies::check_call_site_target_value(klassOop ctxk, oop call_site, oop method_handle, CallSiteDepChange* changes) {
+klassOop Dependencies::check_call_site_target_value(oop call_site, oop method_handle, CallSiteDepChange* changes) {
   assert(call_site    ->is_a(SystemDictionary::CallSite_klass()),     "sanity");
   assert(method_handle->is_a(SystemDictionary::MethodHandle_klass()), "sanity");
   if (changes == NULL) {
     // Validate all CallSites
     if (java_lang_invoke_CallSite::target(call_site) != method_handle)
-      return ctxk;  // assertion failed
+      return call_site->klass();  // assertion failed
   } else {
     // Validate the given CallSite
     if (call_site == changes->call_site() && java_lang_invoke_CallSite::target(call_site) != changes->method_handle()) {
       assert(method_handle != changes->method_handle(), "must be");
-      return ctxk;  // assertion failed
+      return call_site->klass();  // assertion failed
     }
   }
-  assert(java_lang_invoke_CallSite::target(call_site) == method_handle, "should still be valid");
   return NULL;  // assertion still valid
 }
 
@@ -1488,7 +1516,7 @@ klassOop Dependencies::DepStream::check_call_site_dependency(CallSiteDepChange*
   klassOop witness = NULL;
   switch (type()) {
   case call_site_target_value:
-    witness = check_call_site_target_value(context_type(), argument(1), argument(2), changes);
+    witness = check_call_site_target_value(argument(0), argument(1), changes);
     break;
   default:
     witness = NULL;

hotspot/src/share/vm/code/dependencies.hpp

@@ -166,9 +166,14 @@ class Dependencies: public ResourceObj {
     LG2_TYPE_LIMIT = 4,  // assert(TYPE_LIMIT <= (1<<LG2_TYPE_LIMIT))
 
     // handy categorizations of dependency types:
-    all_types      = ((1<<TYPE_LIMIT)-1) & ((-1)<<FIRST_TYPE),
+    all_types           = ((1 << TYPE_LIMIT) - 1) & ((-1) << FIRST_TYPE),
-    non_ctxk_types = (1<<evol_method),
+
-    ctxk_types     = all_types & ~non_ctxk_types,
+    non_klass_types     = (1 << call_site_target_value),
+    klass_types         = all_types & ~non_klass_types,
+
+    non_ctxk_types      = (1 << evol_method),
+    implicit_ctxk_types = (1 << call_site_target_value),
+    explicit_ctxk_types = all_types & ~(non_ctxk_types | implicit_ctxk_types),
 
     max_arg_count = 3,   // current maximum number of arguments (incl. ctxk)
 
@@ -184,9 +189,15 @@ class Dependencies: public ResourceObj {
 
   static const char* dep_name(DepType dept);
   static int         dep_args(DepType dept);
-  static int  dep_context_arg(DepType dept) {
+
-    return dept_in_mask(dept, ctxk_types)? 0: -1;
+  static bool is_klass_type(           DepType dept) { return dept_in_mask(dept, klass_types        ); }
-  }
+
+  static bool has_explicit_context_arg(DepType dept) { return dept_in_mask(dept, explicit_ctxk_types); }
+  static bool has_implicit_context_arg(DepType dept) { return dept_in_mask(dept, implicit_ctxk_types); }
+
+  static int           dep_context_arg(DepType dept) { return has_explicit_context_arg(dept) ? 0 : -1; }
+  static int  dep_implicit_context_arg(DepType dept) { return has_implicit_context_arg(dept) ? 0 : -1; }
+
   static void check_valid_dependency_type(DepType dept);
 
  private:
@@ -250,8 +261,8 @@ class Dependencies: public ResourceObj {
   }
 
   void assert_common_1(DepType dept, ciObject* x);
-  void assert_common_2(DepType dept, ciKlass* ctxk, ciObject* x);
+  void assert_common_2(DepType dept, ciObject* x0, ciObject* x1);
-  void assert_common_3(DepType dept, ciKlass* ctxk, ciObject* x, ciObject* x2);
+  void assert_common_3(DepType dept, ciKlass* ctxk, ciObject* x1, ciObject* x2);
 
  public:
   // Adding assertions to a new dependency set at compile time:
@@ -264,7 +275,7 @@ class Dependencies: public ResourceObj {
   void assert_abstract_with_exclusive_concrete_subtypes(ciKlass* ctxk, ciKlass* k1, ciKlass* k2);
   void assert_exclusive_concrete_methods(ciKlass* ctxk, ciMethod* m1, ciMethod* m2);
   void assert_has_no_finalizable_subclasses(ciKlass* ctxk);
-  void assert_call_site_target_value(ciKlass* ctxk, ciCallSite* call_site, ciMethodHandle* method_handle);
+  void assert_call_site_target_value(ciCallSite* call_site, ciMethodHandle* method_handle);
 
   // Define whether a given method or type is concrete.
   // These methods define the term "concrete" as used in this module.
@@ -318,7 +329,7 @@ class Dependencies: public ResourceObj {
   static klassOop check_exclusive_concrete_methods(klassOop ctxk, methodOop m1, methodOop m2,
                                                    KlassDepChange* changes = NULL);
   static klassOop check_has_no_finalizable_subclasses(klassOop ctxk, KlassDepChange* changes = NULL);
-  static klassOop check_call_site_target_value(klassOop ctxk, oop call_site, oop method_handle, CallSiteDepChange* changes = NULL);
+  static klassOop check_call_site_target_value(oop call_site, oop method_handle, CallSiteDepChange* changes = NULL);
   // A returned klassOop is NULL if the dependency assertion is still
   // valid.  A non-NULL klassOop is a 'witness' to the assertion
   // failure, a point in the class hierarchy where the assertion has
@@ -455,6 +466,8 @@ class Dependencies: public ResourceObj {
     oop argument(int i);         // => recorded_oop_at(argument_index(i))
     klassOop context_type();
 
+    bool is_klass_type()         { return Dependencies::is_klass_type(type()); }
+
     methodOop method_argument(int i) {
       oop x = argument(i);
       assert(x->is_method(), "type");

hotspot/src/share/vm/memory/universe.cpp

@@ -1203,12 +1203,12 @@ void Universe::flush_dependents_on(Handle call_site, Handle method_handle) {
   // Compute the dependent nmethods that have a reference to a
   // CallSite object.  We use instanceKlass::mark_dependent_nmethod
   // directly instead of CodeCache::mark_for_deoptimization because we
-  // want dependents on the class CallSite only not all classes in the
+  // want dependents on the call site class only not all classes in
-  // ContextStream.
+  // the ContextStream.
   int marked = 0;
   {
     MutexLockerEx mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
-    instanceKlass* call_site_klass = instanceKlass::cast(SystemDictionary::CallSite_klass());
+    instanceKlass* call_site_klass = instanceKlass::cast(call_site->klass());
     marked = call_site_klass->mark_dependent_nmethods(changes);
   }
   if (marked > 0) {

hotspot/src/share/vm/opto/callGenerator.cpp

@@ -336,7 +336,7 @@ CallGenerator* CallGenerator::for_direct_call(ciMethod* m, bool separate_io_proj
 }
 
 CallGenerator* CallGenerator::for_dynamic_call(ciMethod* m) {
-  assert(m->is_method_handle_invoke(), "for_dynamic_call mismatch");
+  assert(m->is_method_handle_invoke() || m->is_method_handle_adapter(), "for_dynamic_call mismatch");
   return new DynamicCallGenerator(m);
 }
 
@@ -715,9 +715,9 @@ CallGenerator* CallGenerator::for_method_handle_inline(Node* method_handle, JVMS
     // Get an adapter for the MethodHandle.
     ciMethod* target_method = method_handle->get_method_handle_adapter();
     if (target_method != NULL) {
-      CallGenerator* hit_cg = Compile::current()->call_generator(target_method, -1, false, jvms, true, 1);
+      CallGenerator* cg = Compile::current()->call_generator(target_method, -1, false, jvms, true, PROB_ALWAYS);
-      if (hit_cg != NULL && hit_cg->is_inline())
+      if (cg != NULL && cg->is_inline())
-        return hit_cg;
+        return cg;
     }
   } else if (method_handle->Opcode() == Op_Phi && method_handle->req() == 3 &&
              method_handle->in(1)->Opcode() == Op_ConP && method_handle->in(2)->Opcode() == Op_ConP) {
@@ -754,13 +754,13 @@ CallGenerator* CallGenerator::for_invokedynamic_inline(ciCallSite* call_site, JV
   ciMethod* target_method = method_handle->get_invokedynamic_adapter();
   if (target_method != NULL) {
     Compile *C = Compile::current();
-    CallGenerator* hit_cg = C->call_generator(target_method, -1, false, jvms, true, PROB_ALWAYS);
+    CallGenerator* cg = C->call_generator(target_method, -1, false, jvms, true, PROB_ALWAYS);
-    if (hit_cg != NULL && hit_cg->is_inline()) {
+    if (cg != NULL && cg->is_inline()) {
       // Add a dependence for invalidation of the optimization.
       if (call_site->is_mutable_call_site()) {
-        C->dependencies()->assert_call_site_target_value(C->env()->CallSite_klass(), call_site, method_handle);
+        C->dependencies()->assert_call_site_target_value(call_site, method_handle);
       }
-      return hit_cg;
+      return cg;
     }
   }
   return NULL;