From ad78b7fa67ba30cab2e8f496e4c765be15deeca6 Mon Sep 17 00:00:00 2001 From: Thomas Stuefe Date: Wed, 8 May 2024 10:38:09 +0000 Subject: [PATCH] 8331185: Enable compiler memory limits in debug builds Reviewed-by: asmehra, kvn --- .../compiler/compilationMemoryStatistic.cpp | 34 +++++-- src/hotspot/share/compiler/compilerOracle.cpp | 33 +++++++ src/hotspot/share/compiler/compilerOracle.hpp | 1 + .../jtreg/compiler/c2/TestFindNode.java | 5 +- .../TestDeepGraphVerifyIterativeGVN.java | 5 +- .../print/CompileCommandMemLimit.java | 98 ++++++++++++------- .../print/CompileCommandPrintMemStat.java | 29 ++++-- 7 files changed, 154 insertions(+), 51 deletions(-) diff --git a/src/hotspot/share/compiler/compilationMemoryStatistic.cpp b/src/hotspot/share/compiler/compilationMemoryStatistic.cpp index 585d55febb8..196a11fd065 100644 --- a/src/hotspot/share/compiler/compilationMemoryStatistic.cpp +++ b/src/hotspot/share/compiler/compilationMemoryStatistic.cpp @@ -181,10 +181,16 @@ class MemStatEntry : public CHeapObj { 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; @@ -192,7 +198,7 @@ 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) { } @@ -200,6 +206,7 @@ public: 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,6 +225,7 @@ public: 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"); @@ -225,7 +233,7 @@ public: } 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 @@ public: 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 @@ public: 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 @@ public: 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 index ea09e3ebb7f..87d879feac0 100644 --- 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 @@ public: } }; +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 index 43a30b30fac..f330b3b6075 100644 --- 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 index 91a4cdb8d16..ea7649c713a 100644 --- 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 index 87cbd8620d4..bcb4a211368 100644 --- 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 index c3ac0ae9c81..f1565db47d9 100644 --- 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 @@ import java.util.List; 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 class CompileCommandMemLimit { 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 options = new ArrayList(); 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 @@ public class CompileCommandMemLimit { 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 @@ public class CompileCommandMemLimit { 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); - // Expect this log line - oa.shouldMatch(".*" + expectedNameIncl + ".*Hit MemLimit.*"); + // 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" : "-"; - // Expect final output to contain "oom" - oa.shouldMatch(".*oom.*" + expectedNameIncl + ".*"); + // With C2, we print number of nodes, with C1 we don't + String numberNodesRegex = c2 ? "\\d+" : "-"; + + // method 2 should have hit its tiny limit + oa.shouldMatch(ct + " " + method2regex + ".*: Hit MemLimit \\(limit: 4096 now: \\d+\\)"); + + // 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 class CompileCommandMemLimit { 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 index c0a99863fbf..c2689d88372 100644 --- a/test/hotspot/jtreg/compiler/print/CompileCommandPrintMemStat.java +++ b/test/hotspot/jtreg/compiler/print/CompileCommandPrintMemStat.java @@ -1,5 +1,6 @@ /* - * Copyright (c) 2022, 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 @@ -31,6 +32,7 @@ package compiler.print; +import jdk.test.lib.Platform; import jdk.test.lib.process.OutputAnalyzer; import jdk.test.lib.process.ProcessTools; @@ -68,17 +70,26 @@ public class CompileCommandPrintMemStat { .replace("$", "\\$"); // Should see trace output when methods are compiled - oa.shouldHaveExitValue(0) - .shouldMatch(".*" + expectedNameIncl + ".*") - .shouldNotMatch(".*" + expectedNameExcl + ".*"); + oa.shouldHaveExitValue(0). + shouldMatch("Arena usage.*" + expectedNameIncl + ".*"). + shouldNotMatch("Arena usage.*" + expectedNameExcl + ".*"); + // Should see final report // Looks like this: - // total NA RA result #nodes time type #rc thread method - // 211488 66440 77624 ok 13 0.057 c2 2 0x00007fb49428db70 compiler/print/CompileCommandPrintMemStat$TestMain::method1(()V) - oa.shouldMatch("total.*method"); - oa.shouldMatch("\\d+ +\\d+ +\\d+ +\\S+ +\\d+.*" + expectedNameIncl + ".*"); - oa.shouldNotMatch("\\d+ +\\d+ +\\d+ +\\S+ +\\d+.*" + expectedNameExcl + ".*"); + // total NA RA result #nodes limit time type #rc thread method + // 2149912 0 1986272 ok - - 0.101 c1 1 0x000000015180a600 jdk/internal/org/objectweb/asm/Frame::execute((IILjdk/internal/org/objectweb/asm/Symbol;Ljdk/internal/org/objectweb/asm/SymbolTable;)V) oa.shouldMatch("total.*method"); + // or + // 537784 98184 208536 ok 267 - 0.096 c2 1 0x0000000153019c00 jdk/internal/classfile/impl/BufWriterImpl::writeU1((I)V) 4521912 0 1986272 ok - - 0.101 c1 1 0x000000015180a600 jdk/internal/org/objectweb/asm/Frame::execute((IILjdk/internal/org/objectweb/asm/Symbol;Ljdk/internal/org/objectweb/asm/SymbolTable;)V) oa.shouldMatch("total.*method"); + oa.shouldMatch("\\d+ +\\d+ +\\d+ +ok +(\\d+|-) +.*" + expectedNameIncl + ".*"); + + // In debug builds, we have a default memory limit enabled. That implies MemStat. Therefore we + // expect to see all methods, not just the one we specified on the command line. + if (Platform.isDebugBuild()) { + oa.shouldMatch("\\d+ +\\d+ +\\d+ +ok +(\\d+|-) +.*" + expectedNameExcl + ".*"); + } else { + oa.shouldNotMatch(".*" + expectedNameExcl + ".*"); + } } // Test class that is invoked by the sub process