From d936c3024acf428df6d1fb3064a1d8aa5038d277 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Markus=20Gr=C3=B6nlund?= Date: Wed, 18 May 2022 09:06:14 +0000 Subject: [PATCH] 8280844: Epoch shift synchronization point for Compiler threads is inadequate Reviewed-by: egahlin --- .../build/tools/jfr/GenerateJfrFiles.java | 5 --- src/hotspot/share/compiler/compilerEvent.cpp | 16 +++++++- src/hotspot/share/jfr/metadata/metadata.xml | 4 +- src/hotspot/share/jfr/metadata/metadata.xsd | 1 - .../jfr/support/jfrEpochSynchronization.cpp | 39 ------------------- .../jfr/support/jfrEpochSynchronization.hpp | 39 ------------------- 6 files changed, 16 insertions(+), 88 deletions(-) delete mode 100644 src/hotspot/share/jfr/support/jfrEpochSynchronization.cpp delete mode 100644 src/hotspot/share/jfr/support/jfrEpochSynchronization.hpp diff --git a/make/src/classes/build/tools/jfr/GenerateJfrFiles.java b/make/src/classes/build/tools/jfr/GenerateJfrFiles.java index a3b17cf8c3f..712487ce32b 100644 --- a/make/src/classes/build/tools/jfr/GenerateJfrFiles.java +++ b/make/src/classes/build/tools/jfr/GenerateJfrFiles.java @@ -739,7 +739,6 @@ public class GenerateJfrFiles { out.write("#include \"utilities/ticks.hpp\""); out.write("#if INCLUDE_JFR"); out.write("#include \"jfr/recorder/service/jfrEvent.hpp\""); - out.write("#include \"jfr/support/jfrEpochSynchronization.hpp\""); out.write("/*"); out.write(" * Each event class has an assert member function verify() which is invoked"); out.write(" * just before the engine writes the event and its fields to the data stream."); @@ -869,10 +868,6 @@ public class GenerateJfrFiles { if (type.isEvent && type.internal) { out.write(" JfrEventSetting::unhide_internal_types();"); } - if (("_thread_in_native").equals(type.commitState)) { - out.write(" // explicit epoch synchronization check"); - out.write(" JfrEpochSynchronization sync;"); - } for (FieldElement field : type.fields) { if (field.struct) { out.write(" _" + field.name + ".writeData(w);"); diff --git a/src/hotspot/share/compiler/compilerEvent.cpp b/src/hotspot/share/compiler/compilerEvent.cpp index f89db6ce389..6de747d44b3 100644 --- a/src/hotspot/share/compiler/compilerEvent.cpp +++ b/src/hotspot/share/compiler/compilerEvent.cpp @@ -27,7 +27,9 @@ #include "jfr/jfr.hpp" #include "jfr/jfrEvents.hpp" #include "jfr/metadata/jfrSerializer.hpp" +#include "runtime/interfaceSupport.inline.hpp" #include "runtime/semaphore.inline.hpp" +#include "runtime/thread.inline.hpp" #include "utilities/growableArray.hpp" // Synchronizes access to phases_names. @@ -114,6 +116,16 @@ int CompilerEvent::PhaseEvent::get_phase_id(const char* phase_name, bool may_exi return index; } +// As part of event commit, a Method* is tagged as a function of an epoch. +// Epochs evolve during safepoints. To ensure the event is tagged in the correct epoch, +// that is, to avoid a race, the thread will participate in the safepoint protocol +// by transitioning from _thread_in_native to _thread_in_vm. +template +static inline void commit(EventType& event) { + ThreadInVMfromNative transition(JavaThread::current()); + event.commit(); + } + void CompilerEvent::CompilationEvent::post(EventCompilation& event, int compile_id, CompilerType compiler_type, Method* method, int compile_level, bool success, bool is_osr, int code_size, int inlined_bytecodes) { event.set_compileId(compile_id); event.set_compiler(compiler_type); @@ -123,7 +135,7 @@ void CompilerEvent::CompilationEvent::post(EventCompilation& event, int compile_ event.set_isOsr(is_osr); event.set_codeSize(code_size); event.set_inlinedBytes(inlined_bytecodes); - event.commit(); + commit(event); } void CompilerEvent::CompilationFailureEvent::post(EventCompilationFailure& event, int compile_id, const char* reason) { @@ -147,7 +159,7 @@ void CompilerEvent::InlineEvent::post(EventCompilerInlining& event, int compile_ event.set_succeeded(success); event.set_message(msg); event.set_bci(bci); - event.commit(); + commit(event); } void CompilerEvent::InlineEvent::post(EventCompilerInlining& event, int compile_id, Method* caller, Method* callee, bool success, const char* msg, int bci) { diff --git a/src/hotspot/share/jfr/metadata/metadata.xml b/src/hotspot/share/jfr/metadata/metadata.xml index e6d474d8406..1a4a15a81fa 100644 --- a/src/hotspot/share/jfr/metadata/metadata.xml +++ b/src/hotspot/share/jfr/metadata/metadata.xml @@ -558,7 +558,7 @@ - + @@ -586,7 +586,7 @@ - + diff --git a/src/hotspot/share/jfr/metadata/metadata.xsd b/src/hotspot/share/jfr/metadata/metadata.xsd index 95aa2b94e8c..ffef3d932e3 100644 --- a/src/hotspot/share/jfr/metadata/metadata.xsd +++ b/src/hotspot/share/jfr/metadata/metadata.xsd @@ -72,7 +72,6 @@ - diff --git a/src/hotspot/share/jfr/support/jfrEpochSynchronization.cpp b/src/hotspot/share/jfr/support/jfrEpochSynchronization.cpp deleted file mode 100644 index bc013e56ad6..00000000000 --- a/src/hotspot/share/jfr/support/jfrEpochSynchronization.cpp +++ /dev/null @@ -1,39 +0,0 @@ -/* -* Copyright (c) 2019, 2020, 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. -* -*/ - -#include "precompiled.hpp" -#include "jfr/recorder/checkpoint/types/traceid/jfrTraceIdEpoch.hpp" -#include "jfr/support/jfrEpochSynchronization.hpp" -#include "runtime/interfaceSupport.inline.hpp" -#include "runtime/thread.inline.hpp" - -JfrEpochSynchronization::JfrEpochSynchronization() { - if (JfrTraceIdEpoch::is_synchronizing()) { - // only at a safepoint - JavaThread* const jt = JavaThread::current(); - assert(jt->thread_state() == _thread_in_native, "invariant"); - // use ordinary transition to have the thread block and await the new epoch - ThreadInVMfromNative transition(jt); - } -} diff --git a/src/hotspot/share/jfr/support/jfrEpochSynchronization.hpp b/src/hotspot/share/jfr/support/jfrEpochSynchronization.hpp deleted file mode 100644 index 1e4ede051d2..00000000000 --- a/src/hotspot/share/jfr/support/jfrEpochSynchronization.hpp +++ /dev/null @@ -1,39 +0,0 @@ -/* -* Copyright (c) 2019, 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. -* -*/ - -#ifndef SHARE_JFR_SUPPORT_JFREPOCHSYNCHRONIZATION_HPP -#define SHARE_JFR_SUPPORT_JFREPOCHSYNCHRONIZATION_HPP - -#include "jfr/support/jfrThreadId.hpp" - -/* - * JavaThreads running _thread_in_native (Compiler threads) must synchronize - * with the upcoming epoch in case there is an epoch shift in-progress. - */ -class JfrEpochSynchronization { - public: - JfrEpochSynchronization(); -}; - -#endif // SHARE_JFR_SUPPORT_JFREPOCHSYNCHRONIZATION_HPP