8221183: Avoid code cache walk in MetadataOnStackMark

Note nmethods with "old" Methods in them in table to walk instead.

Reviewed-by: eosterlund, sspitsyn

src/hotspot/share/aot/aotCompiledMethod.hpp

@@ -206,8 +206,6 @@ private:
   // AOT compiled methods do not get into zombie state
   virtual bool can_convert_to_zombie() { return false; }
 
-  // Evol dependent methods already marked.
-  virtual bool is_evol_dependent() { return false; }
   virtual bool is_dependent_on_method(Method* dependee) { return true; }
 
   virtual void clear_inline_caches();

src/hotspot/share/classfile/classLoaderDataGraph.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2018, 2019, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -163,7 +163,7 @@ void ClassLoaderDataGraph::walk_metadata_and_clean_metaspaces() {
   // TODO: have redefinition clean old methods out of the code cache.  They still exist in some places.
   bool walk_all_metadata = InstanceKlass::has_previous_versions_and_reset();
 
-  MetadataOnStackMark md_on_stack(walk_all_metadata);
+  MetadataOnStackMark md_on_stack(walk_all_metadata, /*redefinition_walk*/false);
   clean_deallocate_lists(walk_all_metadata);
 }
 

src/hotspot/share/classfile/metadataOnStackMark.cpp

@@ -50,18 +50,25 @@ class MetadataOnStackClosure : public MetadataClosure {
 // it.  Class unloading only deletes in-error class files, methods created by
 // the relocator and dummy constant pools.  None of these appear anywhere except
 // in metadata Handles.
-MetadataOnStackMark::MetadataOnStackMark(bool redefinition_walk) {
+MetadataOnStackMark::MetadataOnStackMark(bool walk_all_metadata, bool redefinition_walk) {
   assert(SafepointSynchronize::is_at_safepoint(), "sanity check");
   assert(_used_buffers == NULL, "sanity check");
   assert(!_is_active, "MetadataOnStackMarks do not nest");
+  assert(!redefinition_walk || walk_all_metadata,
+         "walk_all_metadata must be true for redefinition_walk");
   NOT_PRODUCT(_is_active = true;)
 
   Threads::metadata_handles_do(Metadata::mark_on_stack);
 
-  if (redefinition_walk) {
+  if (walk_all_metadata) {
     MetadataOnStackClosure md_on_stack;
     Threads::metadata_do(&md_on_stack);
-    CodeCache::metadata_do(&md_on_stack);
+    if (redefinition_walk) {
+      // We have to walk the whole code cache during redefinition.
+      CodeCache::metadata_do(&md_on_stack);
+    } else {
+      CodeCache::old_nmethods_do(&md_on_stack);
+    }
     CompileBroker::mark_on_stack();
     JvmtiCurrentBreakpoints::metadata_do(Metadata::mark_on_stack);
     ThreadService::metadata_do(Metadata::mark_on_stack);

src/hotspot/share/classfile/metadataOnStackMark.hpp

@@ -48,7 +48,7 @@ class MetadataOnStackMark : public StackObj {
   static void retire_buffer(MetadataOnStackBuffer* buffer);
 
  public:
-  MetadataOnStackMark(bool redefinition_walk);
+  MetadataOnStackMark(bool walk_all_metadata, bool redefinition_walk);
    ~MetadataOnStackMark();
 
   static void record(Metadata* m);

src/hotspot/share/code/codeCache.cpp

@@ -1032,43 +1032,77 @@ bool CodeCache::is_far_target(address target) {
 #endif
 }
 
-// Just marks the methods in this class as needing deoptimization
+#ifdef INCLUDE_JVMTI
-void CodeCache::mark_for_evol_deoptimization(InstanceKlass* dependee) {
+// RedefineClasses support for unloading nmethods that are dependent on "old" methods.
-  MutexLockerEx mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
+// We don't really expect this table to grow very large.  If it does, it can become a hashtable.
+static GrowableArray<CompiledMethod*>* old_compiled_method_table = NULL;
 
-  // Deoptimize all methods of the evolving class itself
+static void add_to_old_table(CompiledMethod* c) {
-  Array<Method*>* old_methods = dependee->methods();
+  if (old_compiled_method_table == NULL) {
-  for (int i = 0; i < old_methods->length(); i++) {
+    old_compiled_method_table = new (ResourceObj::C_HEAP, mtCode) GrowableArray<CompiledMethod*>(100, true);
-    ResourceMark rm;
+  }
-    Method* old_method = old_methods->at(i);
+  old_compiled_method_table->push(c);
-    CompiledMethod* nm = old_method->code();
+}
-    if (nm != NULL) {
+
-      nm->mark_for_deoptimization();
+static void reset_old_method_table() {
+  if (old_compiled_method_table != NULL) {
+    delete old_compiled_method_table;
+    old_compiled_method_table = NULL;
+  }
+}
+
+// Remove this method when zombied or unloaded.
+void CodeCache::unregister_old_nmethod(CompiledMethod* c) {
+  assert_locked_or_safepoint(CodeCache_lock);
+  if (old_compiled_method_table != NULL) {
+    int index = old_compiled_method_table->find(c);
+    if (index != -1) {
+      old_compiled_method_table->delete_at(index);
     }
   }
+}
+
+void CodeCache::old_nmethods_do(MetadataClosure* f) {
+  // Walk old method table and mark those on stack.
+  int length = 0;
+  if (old_compiled_method_table != NULL) {
+    length = old_compiled_method_table->length();
+    for (int i = 0; i < length; i++) {
+      old_compiled_method_table->at(i)->metadata_do(f);
+    }
+  }
+  log_debug(redefine, class, nmethod)("Walked %d nmethods for mark_on_stack", length);
+}
+
+// Just marks the methods in this class as needing deoptimization
+void CodeCache::mark_for_evol_deoptimization(InstanceKlass* dependee) {
+  assert(SafepointSynchronize::is_at_safepoint(), "Can only do this at a safepoint!");
 
   // Mark dependent AOT nmethods, which are only found via the class redefined.
+  // TODO: add dependencies to aotCompiledMethod's metadata section so this isn't
+  // needed.
   AOTLoader::mark_evol_dependent_methods(dependee);
 }
 
+
 // Walk compiled methods and mark dependent methods for deoptimization.
 int CodeCache::mark_dependents_for_evol_deoptimization() {
+  assert(SafepointSynchronize::is_at_safepoint(), "Can only do this at a safepoint!");
+  // Each redefinition creates a new set of nmethods that have references to "old" Methods
+  // So delete old method table and create a new one.
+  reset_old_method_table();
+
   int number_of_marked_CodeBlobs = 0;
   CompiledMethodIterator iter(CompiledMethodIterator::only_alive_and_not_unloading);
   while(iter.next()) {
     CompiledMethod* nm = iter.method();
-    if (nm->is_marked_for_deoptimization()) {
+    // Walk all alive nmethods to check for old Methods.
-      // ...Already marked in the previous pass; count it here.
+    // This includes methods whose inline caches point to old methods, so
-      // Also counts AOT compiled methods, already marked.
+    // inline cache clearing is unnecessary.
-      number_of_marked_CodeBlobs++;
+    if (nm->has_evol_metadata()) {
-    } else if (nm->has_evol_metadata()) {
-      ResourceMark rm;
       nm->mark_for_deoptimization();
+      add_to_old_table(nm);
       number_of_marked_CodeBlobs++;
-    } else {
-      // Inline caches that refer to an nmethod are deoptimized already, because
-      // the Method* is walked in the metadata section of the nmethod.
-      assert(!nm->is_evol_dependent(), "should no longer be necessary");
     }
   }
 
@@ -1077,6 +1111,46 @@ int CodeCache::mark_dependents_for_evol_deoptimization() {
   return number_of_marked_CodeBlobs;
 }
 
+void CodeCache::mark_all_nmethods_for_evol_deoptimization() {
+  assert(SafepointSynchronize::is_at_safepoint(), "Can only do this at a safepoint!");
+  CompiledMethodIterator iter(CompiledMethodIterator::only_alive_and_not_unloading);
+  while(iter.next()) {
+    CompiledMethod* nm = iter.method();
+    if (!nm->method()->is_method_handle_intrinsic()) {
+      nm->mark_for_deoptimization();
+      if (nm->has_evol_metadata()) {
+        add_to_old_table(nm);
+      }
+    }
+  }
+}
+
+// Flushes compiled methods dependent on redefined classes, that have already been
+// marked for deoptimization.
+void CodeCache::flush_evol_dependents() {
+  assert(SafepointSynchronize::is_at_safepoint(), "Can only do this at a safepoint!");
+
+  // CodeCache can only be updated by a thread_in_VM and they will all be
+  // stopped during the safepoint so CodeCache will be safe to update without
+  // holding the CodeCache_lock.
+
+  // At least one nmethod has been marked for deoptimization
+
+  // All this already happens inside a VM_Operation, so we'll do all the work here.
+  // Stuff copied from VM_Deoptimize and modified slightly.
+
+  // We do not want any GCs to happen while we are in the middle of this VM operation
+  ResourceMark rm;
+  DeoptimizationMarker dm;
+
+  // Deoptimize all activations depending on marked nmethods
+  Deoptimization::deoptimize_dependents();
+
+  // Make the dependent methods not entrant
+  make_marked_nmethods_not_entrant();
+}
+#endif // INCLUDE_JVMTI
+
 // Deoptimize all methods
 void CodeCache::mark_all_nmethods_for_deoptimization() {
   MutexLockerEx mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
@@ -1137,32 +1211,6 @@ void CodeCache::flush_dependents_on(InstanceKlass* dependee) {
   }
 }
 
-// Flushes compiled methods dependent on redefined classes, that have already been
-// marked for deoptimization.
-void CodeCache::flush_evol_dependents() {
-  // --- Compile_lock is not held. However we are at a safepoint.
-  assert_locked_or_safepoint(Compile_lock);
-
-  // CodeCache can only be updated by a thread_in_VM and they will all be
-  // stopped during the safepoint so CodeCache will be safe to update without
-  // holding the CodeCache_lock.
-
-  // At least one nmethod has been marked for deoptimization
-
-  // All this already happens inside a VM_Operation, so we'll do all the work here.
-  // Stuff copied from VM_Deoptimize and modified slightly.
-
-  // We do not want any GCs to happen while we are in the middle of this VM operation
-  ResourceMark rm;
-  DeoptimizationMarker dm;
-
-  // Deoptimize all activations depending on marked nmethods
-  Deoptimization::deoptimize_dependents();
-
-  // Make the dependent methods not entrant
-  make_marked_nmethods_not_entrant();
-}
-
 // Flushes compiled methods dependent on dependee
 void CodeCache::flush_dependents_on_method(const methodHandle& m_h) {
   // --- Compile_lock is not held. However we are at a safepoint.

src/hotspot/share/code/codeCache.hpp

@@ -270,10 +270,16 @@ class CodeCache : AllStatic {
 
   // Flushing and deoptimization
   static void flush_dependents_on(InstanceKlass* dependee);
+
+  // RedefineClasses support
   // Flushing and deoptimization in case of evolution
   static void mark_for_evol_deoptimization(InstanceKlass* dependee);
   static int  mark_dependents_for_evol_deoptimization();
+  static void mark_all_nmethods_for_evol_deoptimization();
   static void flush_evol_dependents();
+  static void old_nmethods_do(MetadataClosure* f);
+  static void unregister_old_nmethod(CompiledMethod* c);
+
   // Support for fullspeed debugging
   static void flush_dependents_on_method(const methodHandle& dependee);
 

src/hotspot/share/code/compiledMethod.hpp

@@ -368,7 +368,6 @@ public:
   int  verify_icholder_relocations();
   void verify_oop_relocations();
 
-  virtual bool is_evol_dependent() = 0;
   bool has_evol_metadata();
 
   // Fast breakpoint support. Tells if this compiled method is

src/hotspot/share/code/nmethod.cpp

@@ -1106,6 +1106,7 @@ void nmethod::make_unloaded() {
     MutexLockerEx ml(SafepointSynchronize::is_at_safepoint() ? NULL : CodeCache_lock,
                      Mutex::_no_safepoint_check_flag);
     Universe::heap()->unregister_nmethod(this);
+    CodeCache::unregister_old_nmethod(this);
   }
 
   // Clear the method of this dead nmethod
@@ -1291,6 +1292,7 @@ bool nmethod::make_not_entrant_or_zombie(int state) {
       MutexLockerEx mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
       if (nmethod_needs_unregister) {
         Universe::heap()->unregister_nmethod(this);
+        CodeCache::unregister_old_nmethod(this);
       }
       flush_dependencies(/*delete_immediately*/true);
     }
@@ -1988,32 +1990,6 @@ bool nmethod::check_dependency_on(DepChange& changes) {
   return found_check;
 }
 
-bool nmethod::is_evol_dependent() {
-  for (Dependencies::DepStream deps(this); deps.next(); ) {
-    if (deps.type() == Dependencies::evol_method) {
-      Method* method = deps.method_argument(0);
-      if (method->is_old()) {
-        if (log_is_enabled(Debug, redefine, class, nmethod)) {
-          ResourceMark rm;
-          log_debug(redefine, class, nmethod)
-            ("Found evol dependency of nmethod %s.%s(%s) compile_id=%d on method %s.%s(%s)",
-             _method->method_holder()->external_name(),
-             _method->name()->as_C_string(),
-             _method->signature()->as_C_string(),
-             compile_id(),
-             method->method_holder()->external_name(),
-             method->name()->as_C_string(),
-             method->signature()->as_C_string());
-        }
-        if (TraceDependencies || LogCompilation)
-          deps.log_dependency(method->method_holder());
-        return true;
-      }
-    }
-  }
-  return false;
-}
-
 // Called from mark_for_deoptimization, when dependee is invalidated.
 bool nmethod::is_dependent_on_method(Method* dependee) {
   for (Dependencies::DepStream deps(this); deps.next(); ) {

src/hotspot/share/code/nmethod.hpp

@@ -565,11 +565,6 @@ public:
   // and the changes have invalidated it
   bool check_dependency_on(DepChange& changes);
 
-  // Evolution support. Tells if this compiled method is dependent on any of
-  // redefined methods, such that if m() is replaced,
-  // this compiled method will have to be deoptimized.
-  bool is_evol_dependent();
-
   // Fast breakpoint support. Tells if this compiled method is
   // dependent on the given method. Returns true if this nmethod
   // corresponds to the given method as well.

src/hotspot/share/prims/jvmtiRedefineClasses.cpp

@@ -207,7 +207,7 @@ void VM_RedefineClasses::doit() {
 
   // Mark methods seen on stack and everywhere else so old methods are not
   // cleaned up if they're on the stack.
-  MetadataOnStackMark md_on_stack(true);
+  MetadataOnStackMark md_on_stack(/*walk_all_metadata*/true, /*redefinition_walk*/true);
   HandleMark hm(thread);   // make sure any handles created are deleted
                            // before the stack walk again.
 
@@ -3842,7 +3842,7 @@ void VM_RedefineClasses::flush_dependent_code() {
   // This is the first redefinition, mark all the nmethods for deoptimization
   if (!JvmtiExport::all_dependencies_are_recorded()) {
     log_debug(redefine, class, nmethod)("Marked all nmethods for deopt");
-    CodeCache::mark_all_nmethods_for_deoptimization();
+    CodeCache::mark_all_nmethods_for_evol_deoptimization();
     deopt_needed = true;
   } else {
     int deopt = CodeCache::mark_dependents_for_evol_deoptimization();