8294609: C2: Improve inlining of methods with unloaded signature classes

Reviewed-by: kvn, dlong

src/hotspot/share/ci/ciMethod.cpp

@@ -1161,17 +1161,12 @@ bool ciMethod::was_executed_more_than(int times) {
 // ------------------------------------------------------------------
 // ciMethod::has_unloaded_classes_in_signature
 bool ciMethod::has_unloaded_classes_in_signature() {
-  VM_ENTRY_MARK;
+  // ciSignature is resolved against some accessing class and
-  {
+  // signature classes aren't required to be local. As a benefit,
-    ExceptionMark em(THREAD);
+  // it makes signature classes visible through loader constraints.
-    methodHandle m(THREAD, get_Method());
+  // So, encountering an unloaded class signals it is absent both in
-    bool has_unloaded = Method::has_unloaded_classes_in_signature(m, thread);
+  // the callee (local) and caller contexts.
-    if( HAS_PENDING_EXCEPTION ) {
+  return signature()->has_unloaded_classes();
-      CLEAR_PENDING_EXCEPTION;
-      return true;     // Declare that we may have unloaded classes
-    }
-    return has_unloaded;
-  }
 }
 
 // ------------------------------------------------------------------

src/hotspot/share/ci/ciSignature.cpp

@@ -25,6 +25,7 @@
 #include "precompiled.hpp"
 #include "ci/ciMethodType.hpp"
 #include "ci/ciSignature.hpp"
+#include "ci/ciStreams.hpp"
 #include "ci/ciUtilities.inline.hpp"
 #include "memory/allocation.inline.hpp"
 #include "memory/resourceArea.hpp"
@@ -46,26 +47,23 @@ ciSignature::ciSignature(ciKlass* accessing_klass, const constantPoolHandle& cpo
   ciEnv* env = CURRENT_ENV;
 
   int size = 0;
-  int count = 0;
   ResourceMark rm(THREAD);
-  Symbol* sh = symbol->get_symbol();
+  for (SignatureStream ss(symbol->get_symbol()); !ss.is_done(); ss.next()) {
-  SignatureStream ss(sh);
-  for (; ; ss.next()) {
     // Process one element of the signature
-    ciType* type;
+    ciType* type = NULL;
-    if (!ss.is_reference()) {
+    if (ss.is_reference()) {
-      type = ciType::make(ss.type());
-    } else {
       ciSymbol* klass_name = env->get_symbol(ss.as_symbol());
       type = env->get_klass_by_name_impl(_accessing_klass, cpool, klass_name, false);
+    } else {
+      type = ciType::make(ss.type());
     }
     if (ss.at_return_type()) {
       // don't include return type in size calculation
       _return_type = type;
-      break;
+    } else {
+      _types.append(type);
+      size += type->size();
     }
-    _types.append(type);
-    size += type->size();
   }
   _size = size;
 }
@@ -97,6 +95,22 @@ bool ciSignature::equals(ciSignature* that) {
   return true;
 }
 
+// ------------------------------------------------------------------
+// ciSignature::has_unloaded_classes
+//
+// Reports if there are any unloaded classes present in the signature.
+// Each ciSignature when instantiated is resolved against some accessing class
+// and the resolved classes aren't required to be local, but can be revealed
+// through loader constraints.
+bool ciSignature::has_unloaded_classes() {
+  for (ciSignatureStream str(this); !str.is_done(); str.next()) {
+    if (!str.type()->is_loaded()) {
+      return true;
+    }
+  }
+  return false;
+}
+
 // ------------------------------------------------------------------
 // ciSignature::print_signature
 void ciSignature::print_signature() {

src/hotspot/share/ci/ciSignature.hpp

@@ -63,6 +63,8 @@ public:
 
   int       arg_size_for_bc(Bytecodes::Code bc)  { return size() + (Bytecodes::has_receiver(bc) ? 1 : 0); }
 
+  bool has_unloaded_classes();
+
   bool equals(ciSignature* that);
 
   void print_signature();

src/hotspot/share/oops/method.cpp

@@ -1730,20 +1730,6 @@ bool Method::load_signature_classes(const methodHandle& m, TRAPS) {
   return sig_is_loaded;
 }
 
-bool Method::has_unloaded_classes_in_signature(const methodHandle& m, TRAPS) {
-  ResourceMark rm(THREAD);
-  for(ResolvingSignatureStream ss(m()); !ss.is_done(); ss.next()) {
-    if (ss.type() == T_OBJECT) {
-      // Do not use ss.is_reference() here, since we don't care about
-      // unloaded array component types.
-      Klass* klass = ss.as_klass_if_loaded(THREAD);
-      assert(!HAS_PENDING_EXCEPTION, "as_klass_if_loaded contract");
-      if (klass == NULL) return true;
-    }
-  }
-  return false;
-}
-
 // Exposed so field engineers can debug VM
 void Method::print_short_name(outputStream* st) const {
   ResourceMark rm;

src/hotspot/share/oops/method.hpp

@@ -965,9 +965,6 @@ public:
   // Resolve all classes in signature, return 'true' if successful
   static bool load_signature_classes(const methodHandle& m, TRAPS);
 
-  // Return if true if not all classes references in signature, including return type, has been loaded
-  static bool has_unloaded_classes_in_signature(const methodHandle& m, TRAPS);
-
   // Printing
   void print_short_name(outputStream* st = tty) const; // prints as klassname::methodname; Exposed so field engineers can debug VM
 #if INCLUDE_JVMTI

test/hotspot/jtreg/compiler/c2/unloaded/TestInlineUnloaded.java

@@ -0,0 +1,227 @@
+/*
+ * Copyright (c) 2022, 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 8294609
+ * @requires vm.compiler2.enabled & vm.flagless
+ *
+ * @library /test/lib
+ *
+ * @build compiler.c2.unloaded.TestInlineUnloaded
+ *
+ * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar launcher.jar
+ *                  compiler.c2.unloaded.TestInlineUnloaded
+ *                  compiler.c2.unloaded.TestInlineUnloaded$Launcher
+ * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar parent.jar
+ *                  compiler.c2.unloaded.TestInlineUnloaded$Parent
+ *                  compiler.c2.unloaded.TestInlineUnloaded$Parent$U
+ *                  compiler.c2.unloaded.TestInlineUnloaded$Parent$TestCase
+ *                  compiler.c2.unloaded.TestInlineUnloaded$Parent$Invoker
+ *                  compiler.c2.unloaded.TestInlineUnloaded$Parent$TestNull
+ *                  compiler.c2.unloaded.TestInlineUnloaded$Parent$TestLoadedRemotely
+ *                  compiler.c2.unloaded.TestInlineUnloaded$Parent$TestUnloaded
+ * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar caller.jar
+ *                  compiler.c2.unloaded.TestInlineUnloaded$Caller
+ *                  compiler.c2.unloaded.TestInlineUnloaded$Caller$TestNull
+ * @run driver jdk.test.lib.helpers.ClassFileInstaller -jar callee.jar
+ *                  compiler.c2.unloaded.TestInlineUnloaded$Callee
+ *                  compiler.c2.unloaded.TestInlineUnloaded$Callee$TestNull
+ *
+ * @run driver compiler.c2.unloaded.TestInlineUnloaded
+ */
+
+package compiler.c2.unloaded;
+
+import jdk.test.lib.JDKToolFinder;
+import jdk.test.lib.process.OutputAnalyzer;
+
+import java.io.IOException;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.function.Consumer;
+
+public class TestInlineUnloaded {
+    static final String THIS_CLASS = TestInlineUnloaded.class.getName();
+
+    public static class Parent {
+        public class U {
+        }
+
+        public interface TestCase {
+            U test(Invoker obj, U arg);
+
+            void testArg(Invoker obj, U arg);
+
+            U testRet(Invoker obj);
+
+            void test(Invoker obj);
+        }
+
+        public interface Invoker {
+            void invokeArg(U obj);
+
+            U invokeRet();
+
+            U invoke(U obj);
+        }
+
+        private static class TestNull implements Runnable {
+            final TestCase test;
+            final Invoker recv;
+
+            public TestNull(TestCase test, Invoker recv) {
+                this.test = test;
+                this.recv = recv;
+            }
+
+            @Override
+            public void run() {
+                test.testArg(recv, null);
+                test.testRet(recv);
+                test.test(recv, null);
+            }
+        }
+
+        public static class TestLoadedRemotely extends TestNull {
+            public TestLoadedRemotely(TestCase test, Invoker recv) throws Exception {
+                super(test, recv);
+                Class.forName(U.class.getName()); // preload in parent context
+            }
+        }
+
+        public static class TestUnloaded extends TestNull {
+            public TestUnloaded(TestCase test, Invoker recv) {
+                super(test, recv);
+            }
+        }
+    }
+
+    public static class Caller {
+        public static class TestNull implements Parent.TestCase {
+            public TestNull() {}
+
+            public Parent.U test(Parent.Invoker obj, Parent.U arg) {
+                return obj.invoke(arg);
+            }
+
+            public void testArg(Parent.Invoker obj, Parent.U arg) {
+                obj.invokeArg(arg);
+            }
+
+            public Parent.U testRet(Parent.Invoker obj) {
+                return obj.invokeRet();
+            }
+
+            public void test(Parent.Invoker obj) {
+                test(obj, null);
+            }
+        }
+    }
+
+    public static class Callee {
+        public static class TestNull implements Parent.Invoker {
+            public void invokeArg(Parent.U obj) {}
+
+            public Parent.U invokeRet() {
+                return null;
+            }
+
+            public Parent.U invoke(Parent.U obj) {
+                return obj;
+            }
+        }
+    }
+
+    public static class Launcher {
+        public static void main(String... args) throws Exception {
+            final String testName = args[0];
+
+            URLClassLoader parentCL = new URLClassLoader("parent", new URL[] { new URL("file:parent.jar") }, ClassLoader.getSystemClassLoader());
+            URLClassLoader callerCL = new URLClassLoader("caller", new URL[] { new URL("file:caller.jar") }, parentCL);
+            URLClassLoader calleeCL = new URLClassLoader("callee", new URL[] { new URL("file:callee.jar") }, parentCL);
+
+            Object caller = Class.forName(THIS_CLASS + "$Caller$TestNull", false, callerCL)
+                                 .getDeclaredConstructor().newInstance();
+            Object callee = Class.forName(THIS_CLASS + "$Callee$TestNull", false, calleeCL)
+                                 .getDeclaredConstructor().newInstance();
+
+            Class<?> testClass = Class.forName(THIS_CLASS + "$Parent$TestCase", false, parentCL);
+            Class<?> invClass  = Class.forName(THIS_CLASS + "$Parent$Invoker",  false, parentCL);
+            Class<?> test      = Class.forName(THIS_CLASS + "$Parent$" + testName, false, parentCL);
+            Runnable r = (Runnable) test.getDeclaredConstructor(testClass, invClass)
+                                       .newInstance(caller, callee);
+
+            for (int i = 0; i < 20_000; i ++) {
+                r.run();
+            }
+        }
+    }
+
+    static void run(String testCaseName, Consumer<OutputAnalyzer> processor) throws IOException {
+        ProcessBuilder pb = new ProcessBuilder();
+
+        pb.command(JDKToolFinder.getJDKTool("java"),
+            "-cp", "launcher.jar",
+            "-XX:+IgnoreUnrecognizedVMOptions", "-showversion",
+            "-XX:-TieredCompilation", "-Xbatch",
+            "-XX:+PrintCompilation", "-XX:+UnlockDiagnosticVMOptions", "-XX:+PrintInlining",
+            "-XX:CompileCommand=quiet", "-XX:CompileCommand=compileonly,*TestNull::run",
+            Launcher.class.getName(), testCaseName);
+
+        System.out.println("Command line: [" + pb.command() + "]");
+
+        OutputAnalyzer analyzer = new OutputAnalyzer(pb.start());
+
+        analyzer.shouldHaveExitValue(0);
+
+        // The test is applicable only to C2 (present in Server VM).
+        analyzer.stderrShouldContain("Server VM");
+
+        analyzer.shouldContain("TestNull::run"); // ensure that relevant method is compiled
+
+        processor.accept(analyzer); // test-specific checks
+    }
+
+    public static void main(String[] args) throws Exception {
+        run("TestUnloaded", output -> {
+            output.shouldMatch("TestNull::testArg .* unloaded signature classes");
+            output.shouldMatch("TestNull::testRet .* unloaded signature classes");
+            output.shouldMatch("TestNull::test .* unloaded signature classes");
+
+            output.shouldNotMatch("TestNull::testArg .* inline");
+            output.shouldNotMatch("TestNull::testRet .* inline");
+            output.shouldNotMatch("TestNull::test .* inline");
+        });
+        run("TestLoadedRemotely", output -> {
+            output.shouldMatch("TestNull::testArg .* inline");
+            output.shouldMatch("TestNull::testRet .* inline");
+            output.shouldMatch("TestNull::test .* inline");
+
+            output.shouldNotMatch("TestNull::testArg .* unloaded signature classes");
+            output.shouldNotMatch("TestNull::testRet .* unloaded signature classes");
+            output.shouldNotMatch("TestNull::test .* unloaded signature classes");
+        });
+    }
+}