From a7580022142fcd36c89343e2b8cfbab9b406a7b3 Mon Sep 17 00:00:00 2001 From: Dean Long <17332032+dean-long@users.noreply.github.com> Date: Tue, 2 Nov 2021 16:51:49 -0700 Subject: [PATCH 1/7] replay failure due to incomplete ciMethodData information --- src/hotspot/share/ci/ciEnv.cpp | 1 + src/hotspot/share/ci/ciMethodData.cpp | 8 +--- src/hotspot/share/ci/ciMethodData.hpp | 10 ----- src/hotspot/share/ci/ciReplay.cpp | 39 ++++++++++++------- src/hotspot/share/ci/ciReplay.hpp | 6 +++ src/hotspot/share/runtime/vmStructs.cpp | 1 - .../jtreg/compiler/ciReplay/TestLambdas.java | 2 +- 7 files changed, 35 insertions(+), 32 deletions(-) diff --git a/src/hotspot/share/ci/ciEnv.cpp b/src/hotspot/share/ci/ciEnv.cpp index 0c6f2b5fa2b4c..9b8d118196b7c 100644 --- a/src/hotspot/share/ci/ciEnv.cpp +++ b/src/hotspot/share/ci/ciEnv.cpp @@ -1643,6 +1643,7 @@ void ciEnv::dump_replay_data_helper(outputStream* out) { NoSafepointVerifier no_safepoint; ResourceMark rm; + out->print_cr("version %d", REPLAY_VERSION); #if INCLUDE_JVMTI out->print_cr("JvmtiExport can_access_local_variables %d", _jvmti_can_access_local_variables); out->print_cr("JvmtiExport can_hotswap_or_post_breakpoint %d", _jvmti_can_hotswap_or_post_breakpoint); diff --git a/src/hotspot/share/ci/ciMethodData.cpp b/src/hotspot/share/ci/ciMethodData.cpp index cdf85bb1b4757..d8b77ab2f5193 100644 --- a/src/hotspot/share/ci/ciMethodData.cpp +++ b/src/hotspot/share/ci/ciMethodData.cpp @@ -49,10 +49,7 @@ ciMethodData::ciMethodData(MethodData* md) _saw_free_extra_data(false), // Initialize the escape information (to "don't know."); _eflags(0), _arg_local(0), _arg_stack(0), _arg_returned(0), - _creation_mileage(0), - _current_mileage(0), _invocation_counter(0), - _backedge_counter(0), _orig(), _parameters(NULL) {} @@ -250,10 +247,7 @@ bool ciMethodData::load_data() { load_remaining_extra_data(); // Note: Extra data are all BitData, and do not need translation. - _creation_mileage = mdo->creation_mileage(); - _current_mileage = MethodData::mileage_of(mdo->method()); _invocation_counter = mdo->invocation_count(); - _backedge_counter = mdo->backedge_count(); _state = mdo->is_mature()? mature_state: immature_state; _eflags = mdo->eflags(); @@ -706,7 +700,7 @@ void ciMethodData::dump_replay_data(outputStream* out) { Method* method = mdo->method(); out->print("ciMethodData "); ciMethod::dump_name_as_ascii(out, method); - out->print(" %d %d", _state, current_mileage()); + out->print(" %d %d", _state, _invocation_counter); // dump the contents of the MDO header as raw data unsigned char* orig = (unsigned char*)&_orig; diff --git a/src/hotspot/share/ci/ciMethodData.hpp b/src/hotspot/share/ci/ciMethodData.hpp index c19bbfc2dc496..783b2b8d94f79 100644 --- a/src/hotspot/share/ci/ciMethodData.hpp +++ b/src/hotspot/share/ci/ciMethodData.hpp @@ -395,16 +395,10 @@ class ciMethodData : public ciMetadata { intx _arg_stack; // bit set of stack-allocatable arguments intx _arg_returned; // bit set of returned arguments - int _creation_mileage; // method mileage at MDO creation - - // Maturity of the oop when the snapshot is taken. - int _current_mileage; - // These counters hold the age of MDO in tiered. In tiered we can have the same method // running at different compilation levels concurrently. So, in order to precisely measure // its maturity we need separate counters. int _invocation_counter; - int _backedge_counter; // Coherent snapshot of original header. MethodData::CompilerCounters _orig; @@ -477,11 +471,7 @@ class ciMethodData : public ciMetadata { bool is_empty() { return _state == empty_state; } bool is_mature() { return _state == mature_state; } - int creation_mileage() { return _creation_mileage; } - int current_mileage() { return _current_mileage; } - int invocation_count() { return _invocation_counter; } - int backedge_count() { return _backedge_counter; } #if INCLUDE_RTM_OPT // return cached value diff --git a/src/hotspot/share/ci/ciReplay.cpp b/src/hotspot/share/ci/ciReplay.cpp index b1f5b82c678ef..fe47559ef0133 100644 --- a/src/hotspot/share/ci/ciReplay.cpp +++ b/src/hotspot/share/ci/ciReplay.cpp @@ -65,7 +65,7 @@ typedef struct _ciMethodDataRecord { const char* _signature; int _state; - int _current_mileage; + int _invocation_counter; intptr_t* _data; char* _orig_data; @@ -115,6 +115,7 @@ class CompileReplay : public StackObj { Handle _protection_domain; bool _protection_domain_initialized; Handle _loader; + int _version; GrowableArray _ci_method_records; GrowableArray _ci_method_data_records; @@ -638,6 +639,11 @@ class CompileReplay : public StackObj { tty->print_cr("# %s", _bufptr); } skip_remaining(); + } else if (strcmp("version", cmd) == 0) { + _version = parse_int("version"); + if (_version > REPLAY_VERSION) { + report_error("unrecognized version"); + } } else if (strcmp("compile", cmd) == 0) { process_compile(CHECK); } else if (strcmp("ciMethod", cmd) == 0) { @@ -827,7 +833,11 @@ class CompileReplay : public StackObj { // collect and record all the needed information for later ciMethodDataRecord* rec = new_ciMethodData(method); rec->_state = parse_int("state"); - rec->_current_mileage = parse_int("current_mileage"); + if (_version < 1) { + parse_int("current_mileage"); + } else { + rec->_invocation_counter = parse_int("invocation_counter"); + } rec->_orig_data = parse_data("orig", rec->_orig_data_length); if (rec->_orig_data == NULL) { @@ -876,17 +886,20 @@ class CompileReplay : public StackObj { void process_instanceKlass(TRAPS) { // just load the referenced class Klass* k = parse_klass(CHECK); - if (!_protection_domain_initialized && k != NULL) { - assert(_protection_domain() == NULL, "must be uninitialized"); - // The first entry is the holder class of the method for which a replay compilation is requested. - // Use the same protection domain to load all subsequent classes in order to resolve all classes - // in signatures of inlinees. This ensures that inlining can be done as stated in the replay file. - _protection_domain = Handle(_thread, k->protection_domain()); - } - // Only initialize the protection domain handle with the protection domain of the very first entry. - // This also ensures that older replay files work. - _protection_domain_initialized = true; + if (_version >= 1) { + if (!_protection_domain_initialized && k != NULL) { + assert(_protection_domain() == NULL, "must be uninitialized"); + // The first entry is the holder class of the method for which a replay compilation is requested. + // Use the same protection domain to load all subsequent classes in order to resolve all classes + // in signatures of inlinees. This ensures that inlining can be done as stated in the replay file. + _protection_domain = Handle(_thread, k->protection_domain()); + } + + // Only initialize the protection domain handle with the protection domain of the very first entry. + // This also ensures that older replay files work. + _protection_domain_initialized = true; + } if (k == NULL) { return; @@ -1413,7 +1426,7 @@ void ciReplay::initialize(ciMethodData* m) { tty->cr(); } else { m->_state = rec->_state; - m->_current_mileage = rec->_current_mileage; + m->_invocation_counter = rec->_invocation_counter; if (rec->_data_length != 0) { assert(m->_data_size + m->_extra_data_size == rec->_data_length * (int)sizeof(rec->_data[0]) || m->_data_size == rec->_data_length * (int)sizeof(rec->_data[0]), "must agree"); diff --git a/src/hotspot/share/ci/ciReplay.hpp b/src/hotspot/share/ci/ciReplay.hpp index e7bd626fea592..187f47497bdda 100644 --- a/src/hotspot/share/ci/ciReplay.hpp +++ b/src/hotspot/share/ci/ciReplay.hpp @@ -131,4 +131,10 @@ class ciReplay { }; +// Replay file format version history +// 0: legacy (no version number) +// 1: first instanceKlass sets protection domain (8275868) +// replace current_mileage with invocation_count (8276095) +#define REPLAY_VERSION 1 // current version, bump up for incompatible changes + #endif // SHARE_CI_CIREPLAY_HPP diff --git a/src/hotspot/share/runtime/vmStructs.cpp b/src/hotspot/share/runtime/vmStructs.cpp index 32f09d22d4101..4df8b3c1df21c 100644 --- a/src/hotspot/share/runtime/vmStructs.cpp +++ b/src/hotspot/share/runtime/vmStructs.cpp @@ -824,7 +824,6 @@ nonstatic_field(ciMethodData, _arg_local, intx) \ nonstatic_field(ciMethodData, _arg_stack, intx) \ nonstatic_field(ciMethodData, _arg_returned, intx) \ - nonstatic_field(ciMethodData, _current_mileage, int) \ nonstatic_field(ciMethodData, _orig, MethodData::CompilerCounters) \ \ nonstatic_field(ciField, _holder, ciInstanceKlass*) \ diff --git a/test/hotspot/jtreg/compiler/ciReplay/TestLambdas.java b/test/hotspot/jtreg/compiler/ciReplay/TestLambdas.java index 06dd56114115a..58771003ccb65 100644 --- a/test/hotspot/jtreg/compiler/ciReplay/TestLambdas.java +++ b/test/hotspot/jtreg/compiler/ciReplay/TestLambdas.java @@ -25,7 +25,7 @@ * @test * @bug 8275670 * @library / /test/lib - * @summary testing of ciReplay with inlining + * @summary testing of ciReplay with nested BoundMethodHandles * @requires vm.flightRecorder != true & vm.compMode != "Xint" & vm.debug == true & vm.compiler2.enabled * @modules java.base/jdk.internal.misc * @build sun.hotspot.WhiteBox From 34e13543459b6c622893070dbd54a94b0866089a Mon Sep 17 00:00:00 2001 From: Dean Long <17332032+dean-long@users.noreply.github.com> Date: Thu, 11 Nov 2021 19:09:45 -0800 Subject: [PATCH 2/7] remove comment --- src/hotspot/share/ci/ciReplay.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/hotspot/share/ci/ciReplay.cpp b/src/hotspot/share/ci/ciReplay.cpp index fe47559ef0133..62c411a690f24 100644 --- a/src/hotspot/share/ci/ciReplay.cpp +++ b/src/hotspot/share/ci/ciReplay.cpp @@ -896,8 +896,6 @@ class CompileReplay : public StackObj { _protection_domain = Handle(_thread, k->protection_domain()); } - // Only initialize the protection domain handle with the protection domain of the very first entry. - // This also ensures that older replay files work. _protection_domain_initialized = true; } From f8e2c9fc232d4ca2864fa9b0d6ffef4f7a317831 Mon Sep 17 00:00:00 2001 From: Dean Long <17332032+dean-long@users.noreply.github.com> Date: Thu, 11 Nov 2021 19:13:42 -0800 Subject: [PATCH 3/7] initialize _version to 0 --- src/hotspot/share/ci/ciReplay.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/hotspot/share/ci/ciReplay.cpp b/src/hotspot/share/ci/ciReplay.cpp index 62c411a690f24..6720b7f10151d 100644 --- a/src/hotspot/share/ci/ciReplay.cpp +++ b/src/hotspot/share/ci/ciReplay.cpp @@ -160,6 +160,7 @@ class CompileReplay : public StackObj { _iklass = NULL; _entry_bci = 0; _comp_level = 0; + _version = 0; test(); } From 20bea8492563eb7570e9b3eb36f01af3e7654ede Mon Sep 17 00:00:00 2001 From: Dean Long <17332032+dean-long@users.noreply.github.com> Date: Thu, 11 Nov 2021 19:32:12 -0800 Subject: [PATCH 4/7] _current_mileage field is never used, stub out access --- .../share/classes/sun/jvm/hotspot/ci/ciMethodData.java | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/ci/ciMethodData.java b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/ci/ciMethodData.java index 2bad7f97a3e67..237eb6227d958 100644 --- a/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/ci/ciMethodData.java +++ b/src/jdk.hotspot.agent/share/classes/sun/jvm/hotspot/ci/ciMethodData.java @@ -46,13 +46,11 @@ public void update(Observable o, Object data) { private static synchronized void initialize(TypeDataBase db) throws WrongTypeException { Type type = db.lookupType("ciMethodData"); origField = type.getField("_orig"); - currentMileageField = new CIntField(type.getCIntegerField("_current_mileage"), 0); argReturnedField = new CIntField(type.getCIntegerField("_arg_returned"), 0); argStackField = new CIntField(type.getCIntegerField("_arg_stack"), 0); argLocalField = new CIntField(type.getCIntegerField("_arg_local"), 0); eflagsField = new CIntField(type.getCIntegerField("_eflags"), 0); hintDiField = new CIntField(type.getCIntegerField("_hint_di"), 0); - currentMileageField = new CIntField(type.getCIntegerField("_current_mileage"), 0); dataField = type.getAddressField("_data"); extraDataSizeField = new CIntField(type.getCIntegerField("_extra_data_size"), 0); dataSizeField = new CIntField(type.getCIntegerField("_data_size"), 0); @@ -63,7 +61,6 @@ private static synchronized void initialize(TypeDataBase db) throws WrongTypeExc } private static Field origField; - private static CIntField currentMileageField; private static CIntField argReturnedField; private static CIntField argStackField; private static CIntField argLocalField; @@ -141,7 +138,7 @@ int state() { } int currentMileage() { - return (int)currentMileageField.getValue(getAddress()); + return 0; } boolean outOfBounds(int dataIndex) { From 1bab9da13fb61629173008a3f5e86bd5f8aaa647 Mon Sep 17 00:00:00 2001 From: Dean Long <17332032+dean-long@users.noreply.github.com> Date: Fri, 12 Nov 2021 12:04:40 -0800 Subject: [PATCH 5/7] updated syntax comment --- src/hotspot/share/ci/ciReplay.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/ci/ciReplay.cpp b/src/hotspot/share/ci/ciReplay.cpp index 6720b7f10151d..ef006e3710c46 100644 --- a/src/hotspot/share/ci/ciReplay.cpp +++ b/src/hotspot/share/ci/ciReplay.cpp @@ -809,7 +809,7 @@ class CompileReplay : public StackObj { rec->_instructions_size = parse_int("instructions_size"); } - // ciMethodData orig * data * oops ( )* methods ( )* + // ciMethodData orig * data * oops ( )* methods ( )* void process_ciMethodData(TRAPS) { Method* method = parse_method(CHECK); if (had_error()) return; From 46fd3fac320e9e49a6e2aa33eda148548418e297 Mon Sep 17 00:00:00 2001 From: Dean Long <17332032+dean-long@users.noreply.github.com> Date: Fri, 12 Nov 2021 12:17:29 -0800 Subject: [PATCH 6/7] turn version error into a warning --- src/hotspot/share/ci/ciReplay.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/hotspot/share/ci/ciReplay.cpp b/src/hotspot/share/ci/ciReplay.cpp index ef006e3710c46..43c0fe985d5c3 100644 --- a/src/hotspot/share/ci/ciReplay.cpp +++ b/src/hotspot/share/ci/ciReplay.cpp @@ -643,7 +643,7 @@ class CompileReplay : public StackObj { } else if (strcmp("version", cmd) == 0) { _version = parse_int("version"); if (_version > REPLAY_VERSION) { - report_error("unrecognized version"); + tty->print_cr("# unrecognized version %d, expected <= %d", _version, REPLAY_VERSION); } } else if (strcmp("compile", cmd) == 0) { process_compile(CHECK); From 0552e47a8d0c9b28fda9b38cfd45fad9ffd04c04 Mon Sep 17 00:00:00 2001 From: Dean Long <17332032+dean-long@users.noreply.github.com> Date: Fri, 12 Nov 2021 12:27:46 -0800 Subject: [PATCH 7/7] strengthen version check --- src/hotspot/share/ci/ciReplay.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/hotspot/share/ci/ciReplay.cpp b/src/hotspot/share/ci/ciReplay.cpp index 43c0fe985d5c3..20f33cc4909dd 100644 --- a/src/hotspot/share/ci/ciReplay.cpp +++ b/src/hotspot/share/ci/ciReplay.cpp @@ -642,8 +642,8 @@ class CompileReplay : public StackObj { skip_remaining(); } else if (strcmp("version", cmd) == 0) { _version = parse_int("version"); - if (_version > REPLAY_VERSION) { - tty->print_cr("# unrecognized version %d, expected <= %d", _version, REPLAY_VERSION); + if (_version < 0 || _version > REPLAY_VERSION) { + tty->print_cr("# unrecognized version %d, expected 0 <= version <= %d", _version, REPLAY_VERSION); } } else if (strcmp("compile", cmd) == 0) { process_compile(CHECK);