8139551: Scalability problem with redefinition - multiple code cache walks

Walk code cache and deoptimize once per redefinition.

Reviewed-by: sspitsyn, dlong

src/hotspot/share/aot/aotCodeHeap.cpp

@@ -690,6 +690,28 @@ bool AOTCodeHeap::is_dependent_method(Klass* dependee, AOTCompiledMethod* aot) {
   return false;
 }
 
+void AOTCodeHeap::mark_evol_dependent_methods(InstanceKlass* dependee) {
+  AOTKlassData* klass_data = find_klass(dependee);
+  if (klass_data == NULL) {
+    return; // no AOT records for this class - no dependencies
+  }
+  if (!dependee->has_passed_fingerprint_check()) {
+    return; // different class
+  }
+
+  int methods_offset = klass_data->_dependent_methods_offset;
+  if (methods_offset >= 0) {
+    address methods_cnt_adr = _dependencies + methods_offset;
+    int methods_cnt = *(int*)methods_cnt_adr;
+    int* indexes = (int*)(methods_cnt_adr + 4);
+    for (int i = 0; i < methods_cnt; ++i) {
+      int code_id = indexes[i];
+      AOTCompiledMethod* aot = _code_to_aot[code_id]._aot;
+      aot->mark_for_deoptimization(false);
+    }
+  }
+}
+
 void AOTCodeHeap::sweep_dependent_methods(int* indexes, int methods_cnt) {
   int marked = 0;
   for (int i = 0; i < methods_cnt; ++i) {

src/hotspot/share/aot/aotCodeHeap.hpp

@@ -244,6 +244,7 @@ public:
   Klass* get_klass_from_got(const char* klass_name, int klass_len, const Method* method);
 
   bool is_dependent_method(Klass* dependee, AOTCompiledMethod* aot);
+  void mark_evol_dependent_methods(InstanceKlass* dependee);
 
   const char* get_name_at(int offset) {
     return _metaspace_names + offset;

src/hotspot/share/aot/aotCompiledMethod.cpp

@@ -428,10 +428,6 @@ address AOTCompiledMethod::call_instruction_address(address pc) const {
   return pltcall->instruction_address();
 }
 
-bool AOTCompiledMethod::is_evol_dependent_on(Klass* dependee) {
-  return !is_aot_runtime_stub() && _heap->is_dependent_method(dependee, this);
-}
-
 void AOTCompiledMethod::clear_inline_caches() {
   assert(SafepointSynchronize::is_at_safepoint(), "cleaning of IC's only allowed at safepoint");
   if (is_zombie()) {

src/hotspot/share/aot/aotCompiledMethod.hpp

@@ -206,7 +206,8 @@ private:
   // AOT compiled methods do not get into zombie state
   virtual bool can_convert_to_zombie() { return false; }
 
-  virtual bool is_evol_dependent_on(Klass* dependee);
+  // 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/aot/aotLoader.cpp

@@ -86,6 +86,14 @@ void AOTLoader::metadata_do(void f(Metadata*)) {
   }
 }
 
+void AOTLoader::mark_evol_dependent_methods(InstanceKlass* dependee) {
+  if (UseAOT) {
+    FOR_ALL_AOT_HEAPS(heap) {
+      (*heap)->mark_evol_dependent_methods(dependee);
+    }
+  }
+}
+
 /**
  * List of core modules for which we search for shared libraries.
  */

src/hotspot/share/aot/aotLoader.hpp

@@ -63,6 +63,7 @@ public:
   static uint64_t get_saved_fingerprint(InstanceKlass* ik) NOT_AOT({ return 0; });
   static void oops_do(OopClosure* f) NOT_AOT_RETURN;
   static void metadata_do(void f(Metadata*)) NOT_AOT_RETURN;
+  static void mark_evol_dependent_methods(InstanceKlass* dependee) NOT_AOT_RETURN;
 
   NOT_PRODUCT( static void print_statistics() NOT_AOT_RETURN; )
 

src/hotspot/share/code/codeCache.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 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
@@ -1200,9 +1200,9 @@ bool CodeCache::is_far_target(address target) {
 #endif
 }
 
-int CodeCache::mark_for_evol_deoptimization(InstanceKlass* dependee) {
+// Just marks the methods in this class as needing deoptimization
+void CodeCache::mark_for_evol_deoptimization(InstanceKlass* dependee) {
   MutexLockerEx mu(CodeCache_lock, Mutex::_no_safepoint_check_flag);
-  int number_of_marked_CodeBlobs = 0;
 
   // Deoptimize all methods of the evolving class itself
   Array<Method*>* old_methods = dependee->methods();
@@ -1212,16 +1212,24 @@ int CodeCache::mark_for_evol_deoptimization(InstanceKlass* dependee) {
     CompiledMethod* nm = old_method->code();
     if (nm != NULL) {
       nm->mark_for_deoptimization();
-      number_of_marked_CodeBlobs++;
     }
   }
 
+  // Mark dependent AOT nmethods, which are only found via the class redefined.
+  AOTLoader::mark_evol_dependent_methods(dependee);
+}
+
+// Walk compiled methods and mark dependent methods for deoptimization.
+int CodeCache::mark_dependents_for_evol_deoptimization() {
+  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()) {
-      // ...Already marked in the previous pass; don't count it again.
+      // ...Already marked in the previous pass; count it here.
-    } else if (nm->is_evol_dependent_on(dependee)) {
+      // Also counts AOT compiled methods, already marked.
+      number_of_marked_CodeBlobs++;
+    } else if (nm->is_evol_dependent()) {
       ResourceMark rm;
       nm->mark_for_deoptimization();
       number_of_marked_CodeBlobs++;
@@ -1231,6 +1239,8 @@ int CodeCache::mark_for_evol_deoptimization(InstanceKlass* dependee) {
     }
   }
 
+  // return total count of nmethods marked for deoptimization, if zero the caller
+  // can skip deoptimization
   return number_of_marked_CodeBlobs;
 }
 
@@ -1294,34 +1304,30 @@ void CodeCache::flush_dependents_on(InstanceKlass* dependee) {
   }
 }
 
-// Flushes compiled methods dependent on dependee when the dependee is redefined
+// Flushes compiled methods dependent on redefined classes, that have already been
-// via RedefineClasses
+// marked for deoptimization.
-void CodeCache::flush_evol_dependents_on(InstanceKlass* ev_k) {
+void CodeCache::flush_evol_dependents() {
   // --- Compile_lock is not held. However we are at a safepoint.
   assert_locked_or_safepoint(Compile_lock);
-  if (number_of_nmethods_with_dependencies() == 0 && !UseAOT) return;
 
   // 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.
 
-  // Compute the dependent nmethods
+  // At least one nmethod has been marked for deoptimization
-  if (mark_for_evol_deoptimization(ev_k) > 0) {
-    // 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.
+  // All this already happens inside a VM_Operation, so we'll do all the work here.
-    // Stuff copied from VM_Deoptimize and modified slightly.
+  // 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
+  // We do not want any GCs to happen while we are in the middle of this VM operation
-    ResourceMark rm;
+  ResourceMark rm;
-    DeoptimizationMarker dm;
+  DeoptimizationMarker dm;
 
-    // Deoptimize all activations depending on marked nmethods
+  // Deoptimize all activations depending on marked nmethods
-    Deoptimization::deoptimize_dependents();
+  Deoptimization::deoptimize_dependents();
 
-    // Make the dependent methods not entrant
+  // Make the dependent methods not entrant
-    make_marked_nmethods_not_entrant();
+  make_marked_nmethods_not_entrant();
-  }
 }
 
 // Flushes compiled methods dependent on dependee

src/hotspot/share/code/codeCache.hpp

@@ -288,7 +288,6 @@ class CodeCache : AllStatic {
   // Deoptimization
  private:
   static int  mark_for_deoptimization(KlassDepChange& changes);
-  static int  mark_for_evol_deoptimization(InstanceKlass* dependee);
 
  public:
   static void mark_all_nmethods_for_deoptimization();
@@ -298,7 +297,9 @@ class CodeCache : AllStatic {
   // Flushing and deoptimization
   static void flush_dependents_on(InstanceKlass* dependee);
   // Flushing and deoptimization in case of evolution
-  static void flush_evol_dependents_on(InstanceKlass* dependee);
+  static void mark_for_evol_deoptimization(InstanceKlass* dependee);
+  static int  mark_dependents_for_evol_deoptimization();
+  static void flush_evol_dependents();
   // Support for fullspeed debugging
   static void flush_dependents_on_method(const methodHandle& dependee);
 

src/hotspot/share/code/compiledMethod.hpp

@@ -367,7 +367,7 @@ public:
   int  verify_icholder_relocations();
   void verify_oop_relocations();
 
-  virtual bool is_evol_dependent_on(Klass* dependee) = 0;
+  virtual bool is_evol_dependent() = 0;
   // 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/code/nmethod.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 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
@@ -2021,30 +2021,26 @@ bool nmethod::check_dependency_on(DepChange& changes) {
   return found_check;
 }
 
-bool nmethod::is_evol_dependent_on(Klass* dependee) {
+bool nmethod::is_evol_dependent() {
-  InstanceKlass *dependee_ik = InstanceKlass::cast(dependee);
-  Array<Method*>* dependee_methods = dependee_ik->methods();
   for (Dependencies::DepStream deps(this); deps.next(); ) {
     if (deps.type() == Dependencies::evol_method) {
       Method* method = deps.method_argument(0);
-      for (int j = 0; j < dependee_methods->length(); j++) {
+      if (method->is_old()) {
-        if (dependee_methods->at(j) == method) {
+        if (log_is_enabled(Debug, redefine, class, nmethod)) {
-          if (log_is_enabled(Debug, redefine, class, nmethod)) {
+          ResourceMark rm;
-            ResourceMark rm;
+          log_debug(redefine, class, nmethod)
-            log_debug(redefine, class, nmethod)
+            ("Found evol dependency of nmethod %s.%s(%s) compile_id=%d on method %s.%s(%s)",
-              ("Found evol dependency of nmethod %s.%s(%s) compile_id=%d on method %s.%s(%s)",
+             _method->method_holder()->external_name(),
-               _method->method_holder()->external_name(),
+             _method->name()->as_C_string(),
-               _method->name()->as_C_string(),
+             _method->signature()->as_C_string(),
-               _method->signature()->as_C_string(),
+             compile_id(),
-               compile_id(),
+             method->method_holder()->external_name(),
-               method->method_holder()->external_name(),
+             method->name()->as_C_string(),
-               method->name()->as_C_string(),
+             method->signature()->as_C_string());
-               method->signature()->as_C_string());
-          }
-          if (TraceDependencies || LogCompilation)
-            deps.log_dependency(dependee);
-          return true;
         }
+        if (TraceDependencies || LogCompilation)
+          deps.log_dependency(method->method_holder());
+        return true;
       }
     }
   }

src/hotspot/share/code/nmethod.hpp

@@ -588,9 +588,9 @@ public:
   bool check_dependency_on(DepChange& changes);
 
   // Evolution support. Tells if this compiled method is dependent on any of
-  // methods m() of class dependee, such that if m() in dependee is replaced,
+  // redefined methods, such that if m() is replaced,
   // this compiled method will have to be deoptimized.
-  bool is_evol_dependent_on(Klass* dependee);
+  bool is_evol_dependent();
 
   // Fast breakpoint support. Tells if this compiled method is
   // dependent on the given method. Returns true if this nmethod

src/hotspot/share/prims/jvmtiRedefineClasses.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 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
@@ -123,6 +123,7 @@ bool VM_RedefineClasses::doit_prologue() {
     _res = JVMTI_ERROR_NULL_POINTER;
     return false;
   }
+
   for (int i = 0; i < _class_count; i++) {
     if (_class_defs[i].klass == NULL) {
       _res = JVMTI_ERROR_INVALID_CLASS;
@@ -209,6 +210,9 @@ void VM_RedefineClasses::doit() {
     redefine_single_class(_class_defs[i].klass, _scratch_classes[i], thread);
   }
 
+  // Flush all compiled code that depends on the classes redefined.
+  flush_dependent_code();
+
   // Clean out MethodData pointing to old Method*
   // Have to do this after all classes are redefined and all methods that
   // are redefined are marked as old.
@@ -3836,30 +3840,43 @@ void VM_RedefineClasses::transfer_old_native_function_registrations(InstanceKlas
 // subsequent calls to RedefineClasses need only throw away code
 // that depends on the class.
 //
-void VM_RedefineClasses::flush_dependent_code(InstanceKlass* ik, TRAPS) {
+
+// First step is to walk the code cache for each class redefined and mark
+// dependent methods.  Wait until all classes are processed to deoptimize everything.
+void VM_RedefineClasses::mark_dependent_code(InstanceKlass* ik) {
   assert_locked_or_safepoint(Compile_lock);
 
   // All dependencies have been recorded from startup or this is a second or
   // subsequent use of RedefineClasses
   if (JvmtiExport::all_dependencies_are_recorded()) {
-    CodeCache::flush_evol_dependents_on(ik);
+    CodeCache::mark_for_evol_deoptimization(ik);
-  } else {
-    CodeCache::mark_all_nmethods_for_deoptimization();
-
-    ResourceMark rm(THREAD);
-    DeoptimizationMarker dm;
-
-    // Deoptimize all activations depending on marked nmethods
-    Deoptimization::deoptimize_dependents();
-
-    // Make the dependent methods not entrant
-    CodeCache::make_marked_nmethods_not_entrant();
-
-    // From now on we know that the dependency information is complete
-    JvmtiExport::set_all_dependencies_are_recorded(true);
   }
 }
 
+void VM_RedefineClasses::flush_dependent_code() {
+  assert(SafepointSynchronize::is_at_safepoint(), "sanity check");
+
+  bool deopt_needed;
+
+  // 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();
+    deopt_needed = true;
+  } else {
+    int deopt = CodeCache::mark_dependents_for_evol_deoptimization();
+    log_debug(redefine, class, nmethod)("Marked %d dependent nmethods for deopt", deopt);
+    deopt_needed = (deopt != 0);
+  }
+
+  if (deopt_needed) {
+    CodeCache::flush_evol_dependents();
+  }
+
+  // From now on we know that the dependency information is complete
+  JvmtiExport::set_all_dependencies_are_recorded(true);
+}
+
 void VM_RedefineClasses::compute_added_deleted_matching_methods() {
   Method* old_method;
   Method* new_method;
@@ -3958,8 +3975,8 @@ void VM_RedefineClasses::redefine_single_class(jclass the_jclass,
   JvmtiBreakpoints& jvmti_breakpoints = JvmtiCurrentBreakpoints::get_jvmti_breakpoints();
   jvmti_breakpoints.clearall_in_class_at_safepoint(the_class);
 
-  // Deoptimize all compiled code that depends on this class
+  // Mark all compiled code that depends on this class
-  flush_dependent_code(the_class, THREAD);
+  mark_dependent_code(the_class);
 
   _old_methods = the_class->methods();
   _new_methods = scratch_class->methods();

src/hotspot/share/prims/jvmtiRedefineClasses.hpp

@@ -493,7 +493,8 @@ class VM_RedefineClasses: public VM_Operation {
          InstanceKlass* scratch_class,
          constantPoolHandle scratch_cp, int scratch_cp_length, TRAPS);
 
-  void flush_dependent_code(InstanceKlass* ik, TRAPS);
+  void flush_dependent_code();
+  void mark_dependent_code(InstanceKlass* ik);
 
   // lock classes to redefine since constant pool merging isn't thread safe.
   void lock_classes();

test/hotspot/jtreg/runtime/RedefineTests/TestMultipleClasses.java

@@ -0,0 +1,120 @@
+/*
+ * Copyright (c) 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
+ * under the terms of the GNU General Public License version 2 only, as
+ * published by the Free Software Foundation.
+ *
+ * This code is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License
+ * version 2 for more details (a copy is included in the LICENSE file that
+ * accompanied this code).
+ *
+ * You should have received a copy of the GNU General Public License version
+ * 2 along with this work; if not, write to the Free Software Foundation,
+ * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA.
+ *
+ * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA
+ * or visit www.oracle.com if you need additional information or have any
+ * questions.
+ */
+
+/*
+ * @test
+ * @bug 8139551
+ * @summary Scalability problem with redefinition - multiple code cache walks
+ * @library /test/lib
+ * @modules java.base/jdk.internal.misc
+ * @modules java.compiler
+ *          java.instrument
+ *          jdk.jartool/sun.tools.jar
+ * @run main RedefineClassHelper
+ * @run main/othervm/timeout=180 -javaagent:redefineagent.jar -XX:CompileThreshold=100 -Xlog:redefine+class+nmethod=debug TestMultipleClasses
+ */
+
+import java.lang.instrument.*;
+import java.lang.reflect.*;
+import jdk.test.lib.compiler.InMemoryJavaCompiler;
+
+public class TestMultipleClasses extends ClassLoader {
+
+    public static String B(int count) {
+        return new String("public class B" + count + " {" +
+                "   public static void compiledMethod() { " +
+                "       try{" +
+                "          Thread.sleep(1); " +
+                "       } catch(InterruptedException ie) {" +
+                "       }" +
+                "   }" +
+                "}");
+    }
+
+    static String newB(int count) {
+        return new String("public class B" + count + " {" +
+                "   public static void compiledMethod() { " +
+                "       System.out.println(\"compiledMethod called " + count + "\");" +
+                "   }" +
+                "}");
+    }
+
+    static int index = 0;
+
+    @Override
+    protected Class<?> findClass(String name) throws ClassNotFoundException {
+        if (name.equals("B" + index)) {
+            byte[] b = InMemoryJavaCompiler.compile(name, B(index));
+            return defineClass(name, b, 0, b.length);
+        } else {
+            return super.findClass(name);
+        }
+    }
+
+    static void runCompiledMethodMethods(Class c, int count) throws Exception {
+        // Run for a while so they compile.
+        Object o = c.newInstance();
+        Method m = c.getMethod("compiledMethod");
+        for (int i = 0; i < count; i++) {
+            m.invoke(o);
+        }
+    }
+
+    public static void main(String[] args) throws Exception {
+
+        final int numberOfClasses = 20;
+        Class[] classes = new Class[numberOfClasses];
+        byte[][] newClass = new byte[numberOfClasses][];
+        ClassDefinition[] defs = new ClassDefinition[numberOfClasses];
+
+        TestMultipleClasses loader = new TestMultipleClasses();
+
+        // Load and start all the classes.
+        for (index = 0; index < numberOfClasses; index++) {
+            String name = new String("B" + index);
+            Class c = loader.findClass(name);
+
+            runCompiledMethodMethods(c, 500);
+            // Make class definition for redefinition
+            classes[index] = c;
+            newClass[index] = InMemoryJavaCompiler.compile(c.getName(), newB(index));
+            defs[index] = new ClassDefinition(c, newClass[index]);
+        }
+
+        long startTime = System.currentTimeMillis();
+
+        // Redefine all classes.
+        RedefineClassHelper.instrumentation.redefineClasses(defs);
+
+        long endTime = System.currentTimeMillis();
+
+        System.out.println("Redefinition took " + (endTime - startTime) + " milliseconds");
+
+        System.gc();
+
+        // Run all new classes.
+        for (index = 0; index < numberOfClasses; index++) {
+            runCompiledMethodMethods(classes[index], 1);
+        }
+    }
+}