8331185: Enable compiler memory limits in debug builds

tstuefe · tstuefe · commit ad78b7fa67ba · 2024-05-08T10:38:09.000Z
Reviewed-by: asmehra, kvn
diff --git a/src/hotspot/share/compiler/compilationMemoryStatistic.cpp b/src/hotspot/share/compiler/compilationMemoryStatistic.cpp
@@ -181,25 +181,32 @@ class MemStatEntry : public CHeapObj<mtInternal> {
   int _num_recomp;
   // Compiling thread. Only for diagnostic purposes. Thread may not be alive anymore.
   const Thread* _thread;
+  // active limit for this compilation, if any
+  size_t _limit;
 
+  // peak usage, bytes, over all arenas
   size_t _total;
+  // usage in node arena when total peaked
   size_t _na_at_peak;
+  // usage in resource area when total peaked
   size_t _ra_at_peak;
+  // number of nodes (c2 only) when total peaked
   unsigned _live_nodes_at_peak;
   const char* _result;
 
 public:
 
   MemStatEntry(FullMethodName method)
     : _method(method), _comptype(compiler_c1),
-      _time(0), _num_recomp(0), _thread(nullptr),
+      _time(0), _num_recomp(0), _thread(nullptr), _limit(0),
       _total(0), _na_at_peak(0), _ra_at_peak(0), _live_nodes_at_peak(0),
       _result(nullptr) {
   }
 
   void set_comptype(CompilerType comptype) { _comptype = comptype; }
   void set_current_time() { _time = os::elapsedTime(); }
   void set_current_thread() { _thread = Thread::current(); }
+  void set_limit(size_t limit) { _limit = limit; }
   void inc_recompilation() { _num_recomp++; }
 
   void set_total(size_t n) { _total = n; }
@@ -218,14 +225,15 @@ class MemStatEntry : public CHeapObj<mtInternal> {
     st->print_cr("  RA     : ...how much in resource areas");
     st->print_cr("  result : Result: 'ok' finished successfully, 'oom' hit memory limit, 'err' compilation failed");
     st->print_cr("  #nodes : ...how many nodes (c2 only)");
+    st->print_cr("  limit  : memory limit, if set");
     st->print_cr("  time   : time of last compilation (sec)");
     st->print_cr("  type   : compiler type");
     st->print_cr("  #rc    : how often recompiled");
     st->print_cr("  thread : compiler thread");
   }
 
   static void print_header(outputStream* st) {
-    st->print_cr("total     NA        RA        result  #nodes  time    type  #rc thread              method");
+    st->print_cr("total     NA        RA        result  #nodes  limit   time    type  #rc thread              method");
   }
 
   void print_on(outputStream* st, bool human_readable) const {
@@ -260,7 +268,19 @@ class MemStatEntry : public CHeapObj<mtInternal> {
     col += 8; st->fill_to(col);
 
     // Number of Nodes when memory peaked
-    st->print("%u ", _live_nodes_at_peak);
+    if (_live_nodes_at_peak > 0) {
+      st->print("%u ", _live_nodes_at_peak);
+    } else {
+      st->print("-");
+    }
+    col += 8; st->fill_to(col);
+
+    // Limit
+    if (_limit > 0) {
+      st->print(PROPERFMT " ", PROPERFMTARGS(_limit));
+    } else {
+      st->print("-");
+    }
     col += 8; st->fill_to(col);
 
     // TimeStamp
@@ -322,7 +342,7 @@ class MemStatTable :
 
   void add(const FullMethodName& fmn, CompilerType comptype,
            size_t total, size_t na_at_peak, size_t ra_at_peak,
-           unsigned live_nodes_at_peak, const char* result) {
+           unsigned live_nodes_at_peak, size_t limit, const char* result) {
     assert_lock_strong(NMTCompilationCostHistory_lock);
     MemStatTableKey key(fmn, comptype);
     MemStatEntry** pe = get(key);
@@ -343,6 +363,7 @@ class MemStatTable :
     e->set_na_at_peak(na_at_peak);
     e->set_ra_at_peak(ra_at_peak);
     e->set_live_nodes_at_peak(live_nodes_at_peak);
+    e->set_limit(limit);
     e->set_result(result);
   }
 
@@ -430,6 +451,7 @@ void CompilationMemoryStatistic::on_end_compilation() {
                     arena_stat->na_at_peak(),
                     arena_stat->ra_at_peak(),
                     arena_stat->live_nodes_at_peak(),
+                    arena_stat->limit(),
                     result);
   }
   if (print) {
@@ -521,8 +543,8 @@ void CompilationMemoryStatistic::on_arena_change(ssize_t diff, const Arena* aren
         if (ct != compiler_none && name[0] != '\0') {
           ss.print("%s %s: ", compilertype2name(ct), name);
         }
-        ss.print("Hit MemLimit %s (limit: %zu now: %zu)",
-                 (hit_limit_before ? "again" : ""),
+        ss.print("Hit MemLimit %s(limit: %zu now: %zu)",
+                 (hit_limit_before ? "again " : ""),
                  arena_stat->limit(), arena_stat->peak_since_start());
       }
 
diff --git a/src/hotspot/share/compiler/compilerOracle.cpp b/src/hotspot/share/compiler/compilerOracle.cpp
@@ -42,6 +42,23 @@
 #include "runtime/os.hpp"
 #include "utilities/parseInteger.hpp"
 
+// Default compile commands, if defined, are parsed before any of the
+// explicitly defined compile commands. Thus, explicitly defined compile
+// commands take precedence over default compile commands. The effect is
+// as if the default compile commands had been specified at the start of
+// the command line.
+static const char* const default_compile_commands[] = {
+#ifdef ASSERT
+    // In debug builds, impose a (generous) per-compilation memory limit
+    // to catch pathological compilations during testing. The suboption
+    // "crash" will cause the JVM to assert.
+    //
+    // Note: to disable the default limit at the command line,
+    // set a limit of 0 (e.g. -XX:CompileCommand=MemLimit,*.*,0).
+    "MemLimit,*.*,1G~crash",
+#endif
+    nullptr };
+
 static const char* optiontype_names[] = {
 #define enum_of_types(type, name) name,
         OPTION_TYPES(enum_of_types)
@@ -905,6 +922,14 @@ class LineCopy : StackObj {
     }
 };
 
+bool CompilerOracle::parse_from_line_quietly(char* line) {
+  const bool quiet0 = _quiet;
+  _quiet = true;
+  const bool result = parse_from_line(line);
+  _quiet = quiet0;
+  return result;
+}
+
 bool CompilerOracle::parse_from_line(char* line) {
   if ((line[0] == '\0') || (line[0] == '#')) {
     return true;
@@ -1107,6 +1132,14 @@ bool CompilerOracle::parse_from_string(const char* str, bool (*parse_line)(char*
 
 bool compilerOracle_init() {
   bool success = true;
+  // Register default compile commands first - any commands specified via CompileCommand will
+  // supersede these default commands.
+  for (int i = 0; default_compile_commands[i] != nullptr; i ++) {
+    char* s = os::strdup(default_compile_commands[i]);
+    success = CompilerOracle::parse_from_line_quietly(s);
+    os::free(s);
+    assert(success, "default compile command \"%s\" failed to parse", default_compile_commands[i]);
+  }
   if (!CompilerOracle::parse_from_string(CompileCommand, CompilerOracle::parse_from_line)) {
     success = false;
   }
diff --git a/src/hotspot/share/compiler/compilerOracle.hpp b/src/hotspot/share/compiler/compilerOracle.hpp
@@ -179,6 +179,7 @@ class CompilerOracle : AllStatic {
   // Reads from string instead of file
   static bool parse_from_string(const char* option_string, bool (*parser)(char*));
   static bool parse_from_line(char* line);
+  static bool parse_from_line_quietly(char* line);
   static bool parse_compile_only(char* line);
 
   // Fast check if there is any option set that compile control needs to know about
diff --git a/test/hotspot/jtreg/compiler/c2/TestFindNode.java b/test/hotspot/jtreg/compiler/c2/TestFindNode.java
@@ -27,10 +27,13 @@
  * @requires vm.debug == true & vm.flavor == "server"
  * @summary Test which uses some special flags in order to test Node::find() in debug builds which could result in an endless loop or a stack overflow crash.
  *
- * @run main/othervm -Xbatch -XX:CompileCommand=option,*::*,bool,Vectorize,true -XX:+PrintOpto -XX:+TraceLoopOpts compiler.c2.TestFindNode
+ * @run main/othervm -Xbatch -XX:CompileCommand=option,*::*,bool,Vectorize,true -XX:CompileCommand=memlimit,compiler.c2.TestFindNode::*,0
+ *                   -XX:+PrintOpto -XX:+TraceLoopOpts compiler.c2.TestFindNode
  */
 package compiler.c2;
 
+// Note; we disable the implicit memory limit of 1G in debug JVMs until JDK-8331283 is fixed
+
 public class TestFindNode {
     static volatile int[] iArr;
     static volatile int x;
diff --git a/test/hotspot/jtreg/compiler/loopopts/TestDeepGraphVerifyIterativeGVN.java b/test/hotspot/jtreg/compiler/loopopts/TestDeepGraphVerifyIterativeGVN.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2020, 2024, 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
@@ -28,11 +28,14 @@
  * @summary Test which causes a stack overflow segmentation fault with -XX:VerifyIterativeGVN=1 due to a too deep recursion in Node::verify_recur().
  *
  * @run main/othervm/timeout=600 -Xcomp -XX:VerifyIterativeGVN=1 -XX:CompileCommand=compileonly,compiler.loopopts.TestDeepGraphVerifyIterativeGVN::*
+ *                               -XX:CompileCommand=memlimit,compiler.loopopts.TestDeepGraphVerifyIterativeGVN::*,0
  *                               compiler.loopopts.TestDeepGraphVerifyIterativeGVN
  */
 
 package compiler.loopopts;
 
+// Note; we disable the implicit memory limit of 1G in debug JVMs until JDK-8331295 is fixed
+
 public class TestDeepGraphVerifyIterativeGVN
 {
     static volatile int[] iArr;
diff --git a/test/hotspot/jtreg/compiler/print/CompileCommandMemLimit.java b/test/hotspot/jtreg/compiler/print/CompileCommandMemLimit.java
@@ -1,5 +1,6 @@
 /*
- * Copyright (c) 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2023, 2024, Red Hat, Inc. All rights reserved.
+ * Copyright (c) 2023, 2024, 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
@@ -55,6 +56,7 @@
 
 package compiler.print;
 
+import jdk.test.lib.Platform;
 import jdk.test.lib.process.OutputAnalyzer;
 import jdk.test.lib.process.ProcessTools;
 
@@ -64,8 +66,12 @@
 
 public class CompileCommandMemLimit {
 
+    // Method we don't specify; default memlimit should apply
     final static String METHOD1 = "method1";
+    // Method we explicitly limit to 4K limit
     final static String METHOD2 = "method2";
+    // Method for which we explicitly disable a limit on the command line.
+    final static String METHOD3 = "method3";
 
     static boolean c2;
     static boolean test_crash;
@@ -77,27 +83,22 @@ public static void main(String[] args) throws Exception {
             default: throw new RuntimeException("invalid argument");
         }
         c2 = Boolean.parseBoolean(args[1]);
-        test(METHOD1, METHOD2);
-        test(METHOD2, METHOD1);
-    }
-
-    private static void test(String include, String exclude) throws Exception {
-
-        // A method that is known to cost compilers a bit of memory to compile
 
         List<String> options = new ArrayList<String>();
         options.add("-Xcomp");
         options.add("-XX:-Inline");
         options.add("-Xmx100m");
+        options.add("-XX:-CreateCoredumpOnCrash");
         options.add("-XX:CompileCommand=compileonly," + getTestClass() + "::*");
-        // We pass a very small size to guarantee the crash
-        options.add("-XX:CompileCommand=MemStat," + getTestMethod(include) + ",print");
-        if (test_crash) {
-            options.add("-XX:CompileCommand=MemLimit," + getTestMethod(include) + ",4k~crash");
-            options.add("-XX:-CreateCoredumpOnCrash");
-        } else {
-            options.add("-XX:CompileCommand=MemLimit," + getTestMethod(include) + ",4k");
-        }
+
+        // We want a final report
+        options.add("-XX:CompileCommand=MemStat,*.*,print");
+
+        // We limit method 2 to a very small limit that is guaranteed to trigger
+        options.add("-XX:CompileCommand=MemLimit," + getTestMethod(METHOD2) + ",4k" + (test_crash ? "~crash" : ""));
+
+        // We disable any limit set on method 3
+        options.add("-XX:CompileCommand=MemLimit," + getTestMethod(METHOD3) + ",0");
 
         if (c2) {
             options.add("-XX:-TieredCompilation");
@@ -110,20 +111,22 @@ private static void test(String include, String exclude) throws Exception {
 
         oa.reportDiagnosticSummary();
 
-        String expectedNameIncl = getTestMethod(include)
-                .replace('.', '/')
-                .replace("$", "\\$");
-        String expectedNameExcl = getTestMethod(exclude)
-                .replace('.', '/')
-                .replace("$", "\\$");
-
+        String method1regex = testMethodNameForRegex(getTestMethod(METHOD1));
+        String method2regex = testMethodNameForRegex(getTestMethod(METHOD2));
+        String method3regex = testMethodNameForRegex(getTestMethod(METHOD3));
         String ct = c2 ? "c2" : "c1";
 
         if (test_crash) {
             oa.shouldNotHaveExitValue(0);
             oa.shouldMatch("# *Internal Error.*");
-            oa.shouldMatch("# *fatal error: " + ct + " *" + expectedNameIncl + ".*: Hit MemLimit .*limit: 4096.*");
-            oa.shouldNotMatch(".*" + expectedNameExcl + ".*");
+
+            // method 2 should have hit its tiny limit
+            oa.shouldMatch("# *fatal error: " + ct + " *" + method2regex + ".*: Hit MemLimit .*limit: 4096.*");
+
+            // none of the other ones should have hit a limit
+            oa.shouldNotMatch(method1regex + ".*Hit MemLimit");
+            oa.shouldNotMatch(method3regex + ".*Hit MemLimit");
+
             // Make sure we get a non-zero-sized replay file (JDK-8331314)
             oa.shouldContain("# Compiler replay data is saved as:");
             String replayfile = oa.firstMatch("# (\\S+replay_pid\\d+\\.log)", 1);
@@ -137,18 +140,38 @@ private static void test(String include, String exclude) throws Exception {
             if (f.length() == 0) {
                 throw new RuntimeException("Replayfile " + replayfile + " has size 0");
             }
-
         } else {
-            // Should see trace output when methods are compiled
-            oa.shouldHaveExitValue(0)
-                    .shouldMatch(".*" + expectedNameIncl + ".*")
-                    .shouldNotMatch(".*" + expectedNameExcl + ".*");
+            oa.shouldHaveExitValue(0);
+
+            // In debug builds we have an inbuilt MemLimit. It is very high, so we don't expect it to fire in this test.
+            // But it will still show up in the final report.
+            String implicitMemoryLimit = Platform.isDebugBuild() ? "1024M" : "-";
+
+            // With C2, we print number of nodes, with C1 we don't
+            String numberNodesRegex = c2 ? "\\d+" : "-";
 
-            // Expect this log line
-            oa.shouldMatch(".*" + expectedNameIncl + ".*Hit MemLimit.*");
+            // method 2 should have hit its tiny limit
+            oa.shouldMatch(ct + " " + method2regex + ".*: Hit MemLimit \\(limit: 4096 now: \\d+\\)");
 
-            // Expect final output to contain "oom"
-            oa.shouldMatch(".*oom.*" + expectedNameIncl + ".*");
+            // neither of the other ones should have hit a limit
+            oa.shouldNotMatch(method1regex + ".*Hit MemLimit");
+            oa.shouldNotMatch(method3regex + ".*Hit MemLimit");
+
+            // Final report:
+            // Method 1 should show up as "ok" and with the default limit, e.g.
+            // total     NA        RA        result  #nodes  limit   time    type  #rc thread              method
+            // 32728     0         32728     ok     -       1024M   0.045   c1    1   0x000000011b019c10  compiler/print/CompileCommandMemLimit$TestMain::method1(()J)
+            oa.shouldMatch("\\d+ +\\d+ +\\d+ +ok +" + numberNodesRegex + " +" + implicitMemoryLimit + " +.* +" + method1regex);
+
+            // Method 2 should show up as "oom" and with its tiny limit, e.g.
+            // total     NA        RA        result  #nodes  limit   time    type  #rc thread              method
+            // 32728     0         32728     oom     -       4096B   0.045   c1    1   0x000000011b019c10  compiler/print/CompileCommandMemLimit$TestMain::method1(()J)
+            oa.shouldMatch("\\d+ +\\d+ +\\d+ +oom +" + numberNodesRegex + " +4096B +.* +" + method2regex);
+
+            // Method 3 should show up as "ok", and with no limit, even in debug builds, e.g.
+            // total     NA        RA        result  #nodes  limit   time    type  #rc thread              method
+            // 32728     0         32728     ok     -       -        0.045   c1    1   0x000000011b019c10  compiler/print/CompileCommandMemLimit$TestMain::method1(()J)
+            oa.shouldMatch("\\d+ +\\d+ +\\d+ +ok +" + numberNodesRegex + " +- +.* +" + method3regex);
         }
     }
 
@@ -161,16 +184,23 @@ public static String getTestMethod(String method) {
         return getTestClass() + "::" + method;
     }
 
+    private static String testMethodNameForRegex(String m) {
+        return m.replace('.', '/')
+                .replace("$", "\\$");
+    }
+
     public static class TestMain {
         public static void main(String[] args) {
             method1();
             method2();
+            method3();
         }
 
         static long method1() {
             return System.currentTimeMillis();
         }
         static void method2() {}
+        static void method3() {}
     }
 }
 
diff --git a/test/hotspot/jtreg/compiler/print/CompileCommandPrintMemStat.java b/test/hotspot/jtreg/compiler/print/CompileCommandPrintMemStat.java

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,5 @@`
`1`	`1`	`/*`
`2`		`- * Copyright (c) 2020, 2023, Oracle and/or its affiliates. All rights reserved.`
	`2`	`+ * Copyright (c) 2020, 2024, Oracle and/or its affiliates. All rights reserved.`
`3`	`3`	`* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.`
`4`	`4`	`*`
`5`	`5`	`* This code is free software; you can redistribute it and/or modify it`
`@@ -28,11 +28,14 @@`
`28`	`28`	`* @summary Test which causes a stack overflow segmentation fault with -XX:VerifyIterativeGVN=1 due to a too deep recursion in Node::verify_recur().`
`29`	`29`	`*`
`30`	`30`	`* @run main/othervm/timeout=600 -Xcomp -XX:VerifyIterativeGVN=1 -XX:CompileCommand=compileonly,compiler.loopopts.TestDeepGraphVerifyIterativeGVN::*`
	`31`	`+ * -XX:CompileCommand=memlimit,compiler.loopopts.TestDeepGraphVerifyIterativeGVN::*,0`
`31`	`32`	`* compiler.loopopts.TestDeepGraphVerifyIterativeGVN`
`32`	`33`	`*/`
`33`	`34`
`34`	`35`	`package compiler.loopopts;`
`35`	`36`
	`37`	`+// Note; we disable the implicit memory limit of 1G in debug JVMs until JDK-8331295 is fixed`
	`38`	`+`
`36`	`39`	`public class TestDeepGraphVerifyIterativeGVN`
`37`	`40`	`{`
`38`	`41`	`static volatile int[] iArr;`