stan/jdk-24
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

8257621: JFR StringPool misses cached items across consecutive recordings

Browse Source 
Reviewed-by: egahlin
This commit is contained in:
Markus Grönlund 2020-12-17 14:34:44 +00:00
parent 61390d8e45
commit 7aac4dc175
14 changed files with 106 additions and 152 deletions

8
src/hotspot/share/jfr/jni/jfrJniMethod.cpp

@ -160,10 +160,6 @@ NO_TRANSITION(jboolean, jfr_is_available(JNIEnv* env, jclass jvm))
return !Jfr::is_disabled() ? JNI_TRUE : JNI_FALSE;
NO_TRANSITION_END
NO_TRANSITION(jlong, jfr_get_epoch_address(JNIEnv* env, jobject jvm))
return JfrTraceIdEpoch::epoch_address();
NO_TRANSITION_END
NO_TRANSITION(jlong, jfr_get_unloaded_event_classes_count(JNIEnv* env, jobject jvm))
return JfrKlassUnloading::event_class_count();
NO_TRANSITION_END
@ -327,8 +323,8 @@ JVM_ENTRY_NO_ENV(jlong, jfr_type_id(JNIEnv* env, jobject jvm, jclass jc))
return JfrTraceId::load_raw(jc);
JVM_END
JVM_ENTRY_NO_ENV(jboolean, jfr_add_string_constant(JNIEnv* env, jclass jvm, jboolean epoch, jlong id, jstring string))
return JfrStringPool::add(epoch == JNI_TRUE, id, string, thread) ? JNI_TRUE : JNI_FALSE;
JVM_ENTRY_NO_ENV(jboolean, jfr_add_string_constant(JNIEnv* env, jclass jvm, jlong id, jstring string))
return JfrStringPool::add(id, string, thread);
JVM_END
JVM_ENTRY_NO_ENV(void, jfr_set_force_instrumentation(JNIEnv* env, jobject jvm, jboolean force_instrumentation))

4
src/hotspot/share/jfr/jni/jfrJniMethod.hpp

@ -120,9 +120,7 @@ jboolean JNICALL jfr_event_writer_flush(JNIEnv* env, jclass cls, jobject writer,
void JNICALL jfr_flush(JNIEnv* env, jobject jvm);
void JNICALL jfr_abort(JNIEnv* env, jobject jvm, jstring errorMsg);
jlong JNICALL jfr_get_epoch_address(JNIEnv* env, jobject jvm);
jboolean JNICALL jfr_add_string_constant(JNIEnv* env, jclass jvm, jboolean epoch, jlong id, jstring string);
jboolean JNICALL jfr_add_string_constant(JNIEnv* env, jclass jvm, jlong id, jstring string);
void JNICALL jfr_uncaught_exception(JNIEnv* env, jobject jvm, jobject thread, jthrowable throwable);

3
src/hotspot/share/jfr/jni/jfrJniMethodRegistration.cpp

@ -75,8 +75,7 @@ JfrJniMethodRegistration::JfrJniMethodRegistration(JNIEnv* env) {
(char*)"flush", (char*)"()V", (void*)jfr_flush,
(char*)"setRepositoryLocation", (char*)"(Ljava/lang/String;)V", (void*)jfr_set_repository_location,
(char*)"abort", (char*)"(Ljava/lang/String;)V", (void*)jfr_abort,
(char*)"getEpochAddress", (char*)"()J",(void*)jfr_get_epoch_address,
(char*)"addStringConstant", (char*)"(ZJLjava/lang/String;)Z", (void*)jfr_add_string_constant,
(char*)"addStringConstant", (char*)"(JLjava/lang/String;)Z", (void*)jfr_add_string_constant,
(char*)"uncaughtException", (char*)"(Ljava/lang/Thread;Ljava/lang/Throwable;)V", (void*)jfr_uncaught_exception,
(char*)"setForceInstrumentation", (char*)"(Z)V", (void*)jfr_set_force_instrumentation,
(char*)"getUnloadedEventClassCount", (char*)"()J", (void*)jfr_get_unloaded_event_classes_count,

22
src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp

@ -42,6 +42,7 @@
#include "jfr/utilities/jfrBigEndian.hpp"
#include "jfr/utilities/jfrIterator.hpp"
#include "jfr/utilities/jfrLinkedList.inline.hpp"
#include "jfr/utilities/jfrSignal.hpp"
#include "jfr/utilities/jfrThreadIterator.hpp"
#include "jfr/utilities/jfrTypes.hpp"
#include "jfr/writers/jfrJavaEventWriter.hpp"
@ -57,22 +58,7 @@
typedef JfrCheckpointManager::BufferPtr BufferPtr;
static volatile bool constant_pending = false;
static bool is_constant_pending() {
if (Atomic::load_acquire(&constant_pending)) {
Atomic::release_store(&constant_pending, false); // reset
return true;
}
return false;
}
static void set_constant_pending() {
if (!Atomic::load_acquire(&constant_pending)) {
Atomic::release_store(&constant_pending, true);
}
}
static JfrSignal _new_checkpoint;
static JfrCheckpointManager* _instance = NULL;
JfrCheckpointManager& JfrCheckpointManager::instance() {
@ -231,7 +217,7 @@ BufferPtr JfrCheckpointManager::flush(BufferPtr old, size_t used, size_t request
// indicates a lease is being returned
release(old);
// signal completion of a new checkpoint
set_constant_pending();
_new_checkpoint.signal();
return NULL;
}
BufferPtr new_buffer = lease(old, thread, used + requested);
@ -474,7 +460,7 @@ size_t JfrCheckpointManager::flush_type_set() {
elements = ::flush_type_set(thread);
}
}
if (is_constant_pending()) {
if (_new_checkpoint.is_signaled()) {
WriteOperation wo(_chunkwriter);
MutexedWriteOperation mwo(wo);
_thread_local_mspace->iterate(mwo); // current epoch list

3
src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdEpoch.cpp

@ -26,9 +26,9 @@
#include "jfr/recorder/checkpoint/types/traceid/jfrTraceIdEpoch.hpp"
#include "runtime/safepoint.hpp"
JfrSignal JfrTraceIdEpoch::_tag_state;
bool JfrTraceIdEpoch::_epoch_state = false;
bool JfrTraceIdEpoch::_synchronizing = false;
volatile bool JfrTraceIdEpoch::_changed_tag_state = false;
void JfrTraceIdEpoch::begin_epoch_shift() {
assert(SafepointSynchronize::is_at_safepoint(), "invariant");
@ -43,3 +43,4 @@ void JfrTraceIdEpoch::end_epoch_shift() {
OrderAccess::storestore();
_synchronizing = false;
}

21
src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdEpoch.hpp

@ -25,6 +25,7 @@
#ifndef SHARE_JFR_RECORDER_CHECKPOINT_TYPES_TRACEID_JFRTRACEIDEPOCH_HPP
#define SHARE_JFR_RECORDER_CHECKPOINT_TYPES_TRACEID_JFRTRACEIDEPOCH_HPP
#include "jfr/utilities/jfrSignal.hpp"
#include "jfr/utilities/jfrTypes.hpp"
#include "memory/allocation.hpp"
#include "runtime/atomic.hpp"
@ -54,21 +55,13 @@
class JfrTraceIdEpoch : AllStatic {
friend class JfrCheckpointManager;
private:
static JfrSignal _tag_state;
static bool _epoch_state;
static bool _synchronizing;
static volatile bool _changed_tag_state;
static void begin_epoch_shift();
static void end_epoch_shift();
static bool changed_tag_state() {
return Atomic::load_acquire(&_changed_tag_state);
}
static void set_tag_state(bool value) {
Atomic::release_store(&_changed_tag_state, value);
}
public:
static bool epoch() {
return _epoch_state;
@ -115,17 +108,11 @@ class JfrTraceIdEpoch : AllStatic {
}
static bool has_changed_tag_state() {
if (changed_tag_state()) {
set_tag_state(false);
return true;
}
return false;
return _tag_state.is_signaled();
}
static void set_changed_tag_state() {
if (!changed_tag_state()) {
set_tag_state(true);
}
_tag_state.signal();
}
};

10
src/hotspot/share/jfr/recorder/service/jfrRecorderService.cpp

@ -314,9 +314,7 @@ static size_t write_storage(JfrStorage& storage, JfrChunkWriter& chunkwriter) {
}
typedef Content<JfrStringPool, &JfrStringPool::write> StringPool;
typedef Content<JfrStringPool, &JfrStringPool::write_at_safepoint> StringPoolSafepoint;
typedef WriteCheckpointEvent<StringPool> WriteStringPool;
typedef WriteCheckpointEvent<StringPoolSafepoint> WriteStringPoolSafepoint;
static u4 flush_stringpool(JfrStringPool& string_pool, JfrChunkWriter& chunkwriter) {
StringPool sp(string_pool);
@ -330,12 +328,6 @@ static u4 write_stringpool(JfrStringPool& string_pool, JfrChunkWriter& chunkwrit
return invoke(wsp);
}
static u4 write_stringpool_safepoint(JfrStringPool& string_pool, JfrChunkWriter& chunkwriter) {
StringPoolSafepoint sps(string_pool);
WriteStringPoolSafepoint wsps(chunkwriter, sps, TYPE_STRING);
return invoke(wsps);
}
typedef Content<JfrCheckpointManager, &JfrCheckpointManager::flush_type_set> FlushTypeSetFunctor;
typedef WriteContent<FlushTypeSetFunctor> FlushTypeSet;
@ -569,7 +561,7 @@ void JfrRecorderService::safepoint_write() {
assert(SafepointSynchronize::is_at_safepoint(), "invariant");
_checkpoint_manager.begin_epoch_shift();
if (_string_pool.is_modified()) {
write_stringpool_safepoint(_string_pool, _chunkwriter);
write_stringpool(_string_pool, _chunkwriter);
}
_checkpoint_manager.on_rotation();
_storage.write_at_safepoint();

50
src/hotspot/share/jfr/recorder/stringpool/jfrStringPool.cpp

@ -32,6 +32,7 @@
#include "jfr/recorder/stringpool/jfrStringPool.hpp"
#include "jfr/recorder/stringpool/jfrStringPoolWriter.hpp"
#include "jfr/utilities/jfrLinkedList.inline.hpp"
#include "jfr/utilities/jfrSignal.hpp"
#include "jfr/utilities/jfrTypes.hpp"
#include "logging/log.hpp"
#include "runtime/atomic.hpp"
@ -40,44 +41,19 @@
typedef JfrStringPool::BufferPtr BufferPtr;
static JfrStringPool* _instance = NULL;
static uint64_t store_generation = 0;
static uint64_t serialized_generation = 0;
inline void set_generation(uint64_t value, uint64_t* const dest) {
assert(dest != NULL, "invariant");
Atomic::release_store(dest, value);
}
static void increment_store_generation() {
const uint64_t current_serialized = Atomic::load_acquire(&serialized_generation);
const uint64_t current_stored = Atomic::load_acquire(&store_generation);
if (current_serialized == current_stored) {
set_generation(current_serialized + 1, &store_generation);
}
}
static bool increment_serialized_generation() {
const uint64_t current_stored = Atomic::load_acquire(&store_generation);
const uint64_t current_serialized = Atomic::load_acquire(&serialized_generation);
if (current_stored != current_serialized) {
set_generation(current_stored, &serialized_generation);
return true;
}
return false;
}
static JfrSignal _new_string;
bool JfrStringPool::is_modified() {
return increment_serialized_generation();
return _new_string.is_signaled();
}
static JfrStringPool* _instance = NULL;
JfrStringPool& JfrStringPool::instance() {
return *_instance;
}
JfrStringPool* JfrStringPool::create(JfrChunkWriter& cw) {
store_generation = 0;
serialized_generation = 0;
assert(_instance == NULL, "invariant");
_instance = new JfrStringPool(cw);
return _instance;
@ -155,20 +131,16 @@ BufferPtr JfrStringPool::lease(Thread* thread, size_t size /* 0 */) {
return buffer;
}
bool JfrStringPool::add(bool epoch, jlong id, jstring string, JavaThread* jt) {
jboolean JfrStringPool::add(jlong id, jstring string, JavaThread* jt) {
assert(jt != NULL, "invariant");
const bool current_epoch = JfrTraceIdEpoch::epoch();
if (current_epoch != epoch) {
return current_epoch;
}
{
JfrStringPoolWriter writer(jt);
writer.write(id);
writer.write(string);
writer.inc_nof_strings();
}
increment_store_generation();
return current_epoch;
_new_string.signal();
return JNI_TRUE;
}
template <template <typename> class Operation>
@ -224,13 +196,7 @@ size_t JfrStringPool::write() {
return wo.processed();
}
size_t JfrStringPool::write_at_safepoint() {
assert(SafepointSynchronize::is_at_safepoint(), "invariant");
return write();
}
size_t JfrStringPool::clear() {
increment_serialized_generation();
DiscardOperation discard_operation;
ExclusiveDiscardOperation edo(discard_operation);
assert(_mspace->free_list_is_empty(), "invariant");

4
src/hotspot/share/jfr/recorder/stringpool/jfrStringPool.hpp

@ -45,10 +45,10 @@ typedef JfrMemorySpace<JfrStringPool, JfrMspaceRetrieval, JfrLinkedList<JfrStrin
//
class JfrStringPool : public JfrCHeapObj {
public:
static bool add(bool epoch, jlong id, jstring string, JavaThread* jt);
size_t write();
size_t write_at_safepoint();
size_t clear();
static jboolean add(jlong id, jstring string, JavaThread* jt);
typedef JfrStringPoolMspace::Node Buffer;
typedef JfrStringPoolMspace::NodePtr BufferPtr;

51
src/hotspot/share/jfr/utilities/jfrSignal.hpp Normal file

@ -0,0 +1,51 @@
/*
* Copyright (c) 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.
*
*/
#ifndef SHARE_JFR_UTILITIES_JFRSIGNAL_HPP
#define SHARE_JFR_UTILITIES_JFRSIGNAL_HPP
#include "runtime/atomic.hpp"
class JfrSignal {
private:
mutable volatile bool _signaled;
public:
JfrSignal() : _signaled(false) {}
void signal() const {
if (!Atomic::load_acquire(&_signaled)) {
Atomic::release_store(&_signaled, true);
}
}
bool is_signaled() const {
if (Atomic::load_acquire(&_signaled)) {
Atomic::release_store(&_signaled, false); // auto-reset
return true;
}
return false;
}
};
#endif // SHARE_JFR_UTILITIES_JFRSIGNAL_HPP

5
src/jdk.jfr/share/classes/jdk/jfr/internal/EventWriter.java

@ -222,6 +222,10 @@ public final class EventWriter {
notified = false;
}
private void resetStringPool() {
StringPool.reset();
}
private int usedSize() {
return (int) (currentPosition - startPosition);
}
@ -273,6 +277,7 @@ public final class EventWriter {
if (isNotified()) {
resetNotified();
resetStringPool();
reset();
// returning false will trigger restart of the event write attempt
return false;

13
src/jdk.jfr/share/classes/jdk/jfr/internal/JVM.java

@ -488,18 +488,9 @@ public final class JVM {
*
* @param s string constant to be added, not null
*
* @return the current epoch of this insertion attempt
* @return true, if the string was successfully added.
*/
public static native boolean addStringConstant(boolean epoch, long id, String s);
/**
* Gets the address of the jboolean epoch.
*
* The epoch alternates every checkpoint.
*
* @return The address of the jboolean.
*/
public native long getEpochAddress();
public static native boolean addStringConstant(long id, String s);
public native void uncaughtException(Thread thread, Throwable t);

63
src/jdk.jfr/share/classes/jdk/jfr/internal/StringPool.java

@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 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
@ -27,31 +27,23 @@ package jdk.jfr.internal;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.atomic.AtomicLong;
import jdk.internal.misc.Unsafe;
public final class StringPool {
private static final Unsafe unsafe = Unsafe.getUnsafe();
static final int MIN_LIMIT = 16;
static final int MAX_LIMIT = 128; /* 0 MAX means disabled */
private static final long epochAddress;
private static final long DO_NOT_POOL = -1;
private static final SimpleStringIdPool sp = new SimpleStringIdPool();
static {
epochAddress = JVM.getJVM().getEpochAddress();
sp.reset();
}
public static long addString(String s) {
static long addString(String s) {
return sp.addString(s);
}
private static boolean getCurrentEpoch() {
return unsafe.getByte(epochAddress) == 1;
static void reset() {
sp.reset();
}
private static class SimpleStringIdPool {
/* string id index */
private final AtomicLong sidIdx = new AtomicLong(1);
/* epoch of cached strings */
private boolean poolEpoch;
/* the cache */
private final ConcurrentHashMap<String, Long> cache;
/* max size */
@ -60,7 +52,6 @@ public final class StringPool {
private final long MAX_SIZE_UTF16 = 16*1024*1024;
/* max size bytes*/
private long currentSizeUTF16;
/* looking at a biased data set 4 is a good value */
private final String[] preCache = new String[]{"", "" , "" ,""};
/* index of oldest */
@ -69,35 +60,28 @@ public final class StringPool {
private static final int preCacheMask = 0x03;
SimpleStringIdPool() {
cache = new ConcurrentHashMap<>(MAX_SIZE, 0.75f);
this.cache = new ConcurrentHashMap<>(MAX_SIZE, 0.75f);
}
void reset() {
reset(getCurrentEpoch());
}
private void reset(boolean epoch) {
private void reset() {
this.cache.clear();
this.poolEpoch = epoch;
this.currentSizeUTF16 = 0;
synchronized(SimpleStringIdPool.class) {
this.currentSizeUTF16 = 0;
}
}
private long addString(String s) {
boolean currentEpoch = getCurrentEpoch();
if (poolEpoch == currentEpoch) {
/* pool is for current chunk */
Long lsid = this.cache.get(s);
if (lsid != null) {
return lsid.longValue();
}
} else {
/* pool is for an old chunk */
reset(currentEpoch);
Long lsid = this.cache.get(s);
if (lsid != null) {
return lsid.longValue();
}
if (!preCache(s)) {
/* we should not pool this string */
return -1;
return DO_NOT_POOL;
}
if (cache.size() > MAX_SIZE || currentSizeUTF16 > MAX_SIZE_UTF16) {
/* pool was full */
reset(currentEpoch);
reset();
}
return storeString(s);
}
@ -106,14 +90,13 @@ public final class StringPool {
long sid = this.sidIdx.getAndIncrement();
/* we can race but it is ok */
this.cache.put(s, sid);
boolean currentEpoch;
synchronized(SimpleStringIdPool.class) {
currentEpoch = JVM.addStringConstant(poolEpoch, sid, s);
JVM.addStringConstant(sid, s);
currentSizeUTF16 += s.length();
}
/* did we write in chunk that this pool represent */
return currentEpoch == poolEpoch ? sid : -1;
return sid;
}
private boolean preCache(String s) {
if (preCache[0].equals(s)) {
return true;

1
test/jdk/ProblemList.txt

@ -837,7 +837,6 @@ javax/script/Test7.java 8239361 generic-
# jdk_jfr
jdk/jfr/api/recording/event/TestReEnableName.java 8256482 windows-all
jdk/jfr/event/runtime/TestNetworkUtilizationEvent.java 8228990,8229370 generic-all
jdk/jfr/event/compiler/TestCodeSweeper.java 8225209 generic-all
jdk/jfr/event/os/TestThreadContextSwitches.java 8247776 windows-all