changeset 54124:b103107d7ceb

8258414: OldObjectSample events too expensive Reviewed-by: jbachorik Contributed-by: florian.david@datadoghq.com
author mgronlun
date Wed, 21 Apr 2021 12:24:28 +0200
parents d40fd2234553
children 31224d74aa23
files src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleCheckpoint.cpp src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleCheckpoint.hpp src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.cpp src/hotspot/share/jfr/recorder/service/jfrRecorderService.cpp src/hotspot/share/jfr/recorder/stacktrace/jfrStackTraceRepository.cpp src/hotspot/share/jfr/recorder/stacktrace/jfrStackTraceRepository.hpp src/hotspot/share/jfr/support/jfrAllocationTracer.cpp src/hotspot/share/jfr/support/jfrAllocationTracer.hpp
diffstat 8 files changed, 126 insertions(+), 94 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleCheckpoint.cpp	Wed Apr 21 11:01:25 2021 +0200
+++ b/src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleCheckpoint.cpp	Wed Apr 21 12:24:28 2021 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2021, 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
@@ -232,12 +232,16 @@
 
 class StackTraceBlobInstaller {
  private:
-  const JfrStackTraceRepository& _stack_trace_repo;
   BlobCache _cache;
-  const JfrStackTrace* resolve(const ObjectSample* sample);
   void install(ObjectSample* sample);
+  const JfrStackTrace* resolve(const ObjectSample* sample) const;
  public:
-  StackTraceBlobInstaller(const JfrStackTraceRepository& stack_trace_repo);
+  StackTraceBlobInstaller() : _cache(JfrOptionSet::old_object_queue_size()) {
+    prepare_for_resolution();
+  }
+  ~StackTraceBlobInstaller() {
+    JfrStackTraceRepository::clear_leak_profiler();
+  }
   void sample_do(ObjectSample* sample) {
     if (stack_trace_precondition(sample)) {
       install(sample);
@@ -245,15 +249,6 @@
   }
 };
 
-StackTraceBlobInstaller::StackTraceBlobInstaller(const JfrStackTraceRepository& stack_trace_repo) :
-  _stack_trace_repo(stack_trace_repo), _cache(JfrOptionSet::old_object_queue_size()) {
-  prepare_for_resolution();
-}
-
-const JfrStackTrace* StackTraceBlobInstaller::resolve(const ObjectSample* sample) {
-  return _stack_trace_repo.lookup(sample->stack_trace_hash(), sample->stack_trace_id());
-}
-
 #ifdef ASSERT
 static void validate_stack_trace(const ObjectSample* sample, const JfrStackTrace* stack_trace) {
   assert(!sample->has_stacktrace(), "invariant");
@@ -263,6 +258,10 @@
 }
 #endif
 
+inline const JfrStackTrace* StackTraceBlobInstaller::resolve(const ObjectSample* sample) const {
+  return JfrStackTraceRepository::lookup_for_leak_profiler(sample->stack_trace_hash(), sample->stack_trace_id());
+}
+
 void StackTraceBlobInstaller::install(ObjectSample* sample) {
   JfrBlobHandle blob = _cache.get(sample);
   if (blob.valid()) {
@@ -275,25 +274,31 @@
   writer.write_type(TYPE_STACKTRACE);
   writer.write_count(1);
   ObjectSampleCheckpoint::write_stacktrace(stack_trace, writer);
-  blob = writer.move();
+  blob = writer.copy();
   _cache.put(sample, blob);
   sample->set_stacktrace(blob);
 }
 
-static void install_stack_traces(const ObjectSampler* sampler, JfrStackTraceRepository& stack_trace_repo) {
+static void install_stack_traces(const ObjectSampler* sampler) {
   assert(sampler != NULL, "invariant");
   const ObjectSample* const last = sampler->last();
   if (last != sampler->last_resolved()) {
-    StackTraceBlobInstaller installer(stack_trace_repo);
+    ResourceMark rm;
+    StackTraceBlobInstaller installer;
     iterate_samples(installer);
   }
 }
 
 // caller needs ResourceMark
-void ObjectSampleCheckpoint::on_rotation(const ObjectSampler* sampler, JfrStackTraceRepository& stack_trace_repo) {
+void ObjectSampleCheckpoint::on_rotation(const ObjectSampler* sampler) {
   assert(sampler != NULL, "invariant");
   assert(LeakProfiler::is_running(), "invariant");
-  install_stack_traces(sampler, stack_trace_repo);
+  assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint!");
+  JavaThread* const thread = JavaThread::current();
+  DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_native(thread);)
+  // can safepoint here
+  ThreadInVMfromNative transition(thread);
+  install_stack_traces(sampler);
 }
 
 static traceid get_klass_id(traceid method_id) {
--- a/src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleCheckpoint.hpp	Wed Apr 21 11:01:25 2021 +0200
+++ b/src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleCheckpoint.hpp	Wed Apr 21 12:24:28 2021 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2021, 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
@@ -32,7 +32,6 @@
 class JavaThread;
 class JfrCheckpointWriter;
 class JfrStackTrace;
-class JfrStackTraceRepository;
 class Klass;
 class Method;
 class ObjectSample;
@@ -54,7 +53,7 @@
   static void on_type_set(JfrCheckpointWriter& writer);
   static void on_type_set_unload(JfrCheckpointWriter& writer);
   static void on_thread_exit(JavaThread* jt);
-  static void on_rotation(const ObjectSampler* sampler, JfrStackTraceRepository& repo);
+  static void on_rotation(const ObjectSampler* sampler);
 };
 
 #endif // SHARE_VM_LEAKPROFILER_CHECKPOINT_OBJECTSAMPLECHECKPOINT_HPP
--- a/src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.cpp	Wed Apr 21 11:01:25 2021 +0200
+++ b/src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.cpp	Wed Apr 21 12:24:28 2021 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2021, 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
@@ -117,12 +117,23 @@
   return tl->thread_id();
 }
 
-static void record_stacktrace(JavaThread* thread) {
-  assert(thread != NULL, "invariant");
-  if (JfrEventSetting::has_stacktrace(EventOldObjectSample::eventId)) {
-    JfrStackTraceRepository::record_and_cache(thread);
+class RecordStackTrace {
+ private:
+  JavaThread* _jt;
+  bool _enabled;
+ public:
+  RecordStackTrace(JavaThread* jt) : _jt(jt),
+    _enabled(JfrEventSetting::has_stacktrace(EventOldObjectSample::eventId)) {
+    if (_enabled) {
+      JfrStackTraceRepository::record_for_leak_profiler(jt);
+    }
   }
-}
+  ~RecordStackTrace() {
+    if (_enabled) {
+      _jt->jfr_thread_local()->clear_cached_stack_trace();
+    }
+  }
+};
 
 void ObjectSampler::sample(HeapWord* obj, size_t allocated, JavaThread* thread) {
   assert(thread != NULL, "invariant");
@@ -131,7 +142,7 @@
   if (thread_id == 0) {
     return;
   }
-  record_stacktrace(thread);
+  RecordStackTrace rst(thread);
   // try enter critical section
   JfrTryLock tryLock(&_lock);
   if (!tryLock.has_lock()) {
--- a/src/hotspot/share/jfr/recorder/service/jfrRecorderService.cpp	Wed Apr 21 11:01:25 2021 +0200
+++ b/src/hotspot/share/jfr/recorder/service/jfrRecorderService.cpp	Wed Apr 21 12:24:28 2021 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2021, 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
@@ -268,7 +268,7 @@
 }
 
 void JfrRecorderService::pre_safepoint_clear() {
-  _stack_trace_repository.clear();
+  JfrStackTraceRepository::clear();
   _string_pool.clear();
   _storage.clear();
 }
@@ -289,7 +289,7 @@
 //
 void JfrRecorderService::safepoint_clear() {
   assert(SafepointSynchronize::is_at_safepoint(), "invariant");
-  _stack_trace_repository.clear();
+  JfrStackTraceRepository::clear();
   _string_pool.clear();
   _storage.clear();
   _checkpoint_manager.shift_epoch();
@@ -423,7 +423,7 @@
   if (LeakProfiler::is_running()) {
     // Exclusive access to the object sampler instance.
     // The sampler is released (unlocked) later in post_safepoint_write.
-    ObjectSampleCheckpoint::on_rotation(ObjectSampler::acquire(), _stack_trace_repository);
+    ObjectSampleCheckpoint::on_rotation(ObjectSampler::acquire());
   }
   _storage.write();
 }
@@ -449,6 +449,7 @@
 void JfrRecorderService::safepoint_write() {
   assert(SafepointSynchronize::is_at_safepoint(), "invariant");
   MutexLockerEx stream_lock(JfrStream_lock, Mutex::_no_safepoint_check_flag);
+  JfrStackTraceRepository::clear_leak_profiler();
   write_stacktrace_checkpoint(_stack_trace_repository, _chunkwriter, true);
   write_stringpool_checkpoint(_string_pool, _chunkwriter);
   _checkpoint_manager.write_safepoint_types();
--- a/src/hotspot/share/jfr/recorder/stacktrace/jfrStackTraceRepository.cpp	Wed Apr 21 11:01:25 2021 +0200
+++ b/src/hotspot/share/jfr/recorder/stacktrace/jfrStackTraceRepository.cpp	Wed Apr 21 12:24:28 2021 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2021, 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
@@ -30,18 +30,38 @@
 #include "jfr/support/jfrThreadLocal.hpp"
 #include "runtime/mutexLocker.hpp"
 
+/*
+ * There are two separate repository instances.
+ * One instance is dedicated to stacktraces taken as part of the leak profiler subsystem.
+ * It is kept separate because at the point of insertion, it is unclear if a trace will be serialized,
+ * which is a decision postponed and taken during rotation.
+ */
+
 static JfrStackTraceRepository* _instance = NULL;
+static JfrStackTraceRepository* _leak_profiler_instance = NULL;
+static traceid _next_id = 0;
 
-JfrStackTraceRepository::JfrStackTraceRepository() : _next_id(0), _entries(0) {
-  memset(_table, 0, sizeof(_table));
+JfrStackTraceRepository& JfrStackTraceRepository::instance() {
+  assert(_instance != NULL, "invariant");
+  return *_instance;
 }
 
-JfrStackTraceRepository& JfrStackTraceRepository::instance() {
-  return *_instance;
+static JfrStackTraceRepository& leak_profiler_instance() {
+  assert(_leak_profiler_instance != NULL, "invariant");
+  return *_leak_profiler_instance;
+}
+
+JfrStackTraceRepository::JfrStackTraceRepository() : _last_entries(0), _entries(0) {
+  memset(_table, 0, sizeof(_table));
 }
 
 JfrStackTraceRepository* JfrStackTraceRepository::create() {
   assert(_instance == NULL, "invariant");
+  assert(_leak_profiler_instance == NULL, "invariant");
+  _leak_profiler_instance = new JfrStackTraceRepository();
+  if (_leak_profiler_instance == NULL) {
+    return NULL;
+  }
   _instance = new JfrStackTraceRepository();
   return _instance;
 }
@@ -69,9 +89,18 @@
   assert(_instance != NULL, "invarinat");
   delete _instance;
   _instance = NULL;
+  delete _leak_profiler_instance;
+  _leak_profiler_instance = NULL;
 }
 
-size_t JfrStackTraceRepository::write_impl(JfrChunkWriter& sw, bool clear) {
+bool JfrStackTraceRepository::is_modified() const {
+  return _last_entries != _entries;
+}
+
+size_t JfrStackTraceRepository::write(JfrChunkWriter& sw, bool clear) {
+  if (_entries == 0) {
+    return 0;
+  }
   MutexLockerEx lock(JfrStacktrace_lock, Mutex::_no_safepoint_check_flag);
   assert(_entries > 0, "invariant");
   int count = 0;
@@ -93,45 +122,27 @@
     memset(_table, 0, sizeof(_table));
     _entries = 0;
   }
+  _last_entries = _entries;
   return count;
 }
 
-size_t JfrStackTraceRepository::write(JfrChunkWriter& sw, bool clear) {
-  return _entries > 0 ? write_impl(sw, clear) : 0;
-}
-
-traceid JfrStackTraceRepository::write(JfrCheckpointWriter& writer, traceid id, unsigned int hash) {
-  assert(JfrStacktrace_lock->owned_by_self(), "invariant");
-  const JfrStackTrace* const trace = lookup(hash, id);
-  assert(trace != NULL, "invariant");
-  assert(trace->hash() == hash, "invariant");
-  assert(trace->id() == id, "invariant");
-  trace->write(writer);
-  return id;
-}
-
-void JfrStackTraceRepository::write_metadata(JfrCheckpointWriter& writer) {
-  JfrFrameType fct;
-  writer.write_type(TYPE_FRAMETYPE);
-  fct.serialize(writer);
-}
-
-size_t JfrStackTraceRepository::clear() {
+size_t JfrStackTraceRepository::clear(JfrStackTraceRepository& repo) {
   MutexLockerEx lock(JfrStacktrace_lock, Mutex::_no_safepoint_check_flag);
-  if (_entries == 0) {
+  if (repo._entries == 0) {
     return 0;
   }
   for (u4 i = 0; i < TABLE_SIZE; ++i) {
-    JfrStackTrace* stacktrace = _table[i];
+    JfrStackTrace* stacktrace = repo._table[i];
     while (stacktrace != NULL) {
       JfrStackTrace* next = const_cast<JfrStackTrace*>(stacktrace->next());
       delete stacktrace;
       stacktrace = next;
     }
   }
-  memset(_table, 0, sizeof(_table));
-  const size_t processed = _entries;
-  _entries = 0;
+  memset(repo._table, 0, sizeof(repo._table));
+  const size_t processed = repo._entries;
+  repo._entries = 0;
+  repo._last_entries = 0;
   return processed;
 }
 
@@ -157,20 +168,23 @@
 
 traceid JfrStackTraceRepository::record_for(JavaThread* thread, int skip, JfrStackFrame *frames, u4 max_frames) {
   JfrStackTrace stacktrace(frames, max_frames);
-  return stacktrace.record_safe(thread, skip) ? add(stacktrace) : 0;
+  return stacktrace.record_safe(thread, skip) ? add(instance(), stacktrace) : 0;
 }
-
-traceid JfrStackTraceRepository::add(const JfrStackTrace& stacktrace) {
-  traceid tid = instance().add_trace(stacktrace);
+traceid JfrStackTraceRepository::add(JfrStackTraceRepository& repo, const JfrStackTrace& stacktrace) {
+  traceid tid = repo.add_trace(stacktrace);
   if (tid == 0) {
     stacktrace.resolve_linenos();
-    tid = instance().add_trace(stacktrace);
+    tid = repo.add_trace(stacktrace);
   }
   assert(tid != 0, "invariant");
   return tid;
 }
 
-void JfrStackTraceRepository::record_and_cache(JavaThread* thread, int skip /* 0 */) {
+traceid JfrStackTraceRepository::add(const JfrStackTrace& stacktrace) {
+  return add(instance(), stacktrace);
+}
+
+void JfrStackTraceRepository::record_for_leak_profiler(JavaThread* thread, int skip /* 0 */) {
   assert(thread != NULL, "invariant");
   JfrThreadLocal* const tl = thread->jfr_thread_local();
   assert(tl != NULL, "invariant");
@@ -179,7 +193,7 @@
   stacktrace.record_safe(thread, skip);
   const unsigned int hash = stacktrace.hash();
   if (hash != 0) {
-    tl->set_cached_stack_trace_id(instance().add(stacktrace), hash);
+    tl->set_cached_stack_trace_id(add(leak_profiler_instance(), stacktrace), hash);
   }
 }
 
@@ -206,9 +220,9 @@
 }
 
 // invariant is that the entry to be resolved actually exists in the table
-const JfrStackTrace* JfrStackTraceRepository::lookup(unsigned int hash, traceid id) const {
+const JfrStackTrace* JfrStackTraceRepository::lookup_for_leak_profiler(unsigned int hash, traceid id) {
   const size_t index = (hash % TABLE_SIZE);
-  const JfrStackTrace* trace = _table[index];
+  const JfrStackTrace* trace = leak_profiler_instance()._table[index];
   while (trace != NULL && trace->id() != id) {
     trace = trace->next();
   }
@@ -217,3 +231,12 @@
   assert(trace->id() == id, "invariant");
   return trace;
 }
+
+void JfrStackTraceRepository::clear_leak_profiler() {
+  clear(leak_profiler_instance());
+}
+
+size_t JfrStackTraceRepository::clear() {
+  clear_leak_profiler();
+  return clear(instance());
+}
--- a/src/hotspot/share/jfr/recorder/stacktrace/jfrStackTraceRepository.hpp	Wed Apr 21 11:01:25 2021 +0200
+++ b/src/hotspot/share/jfr/recorder/stacktrace/jfrStackTraceRepository.hpp	Wed Apr 21 12:24:28 2021 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2019, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2021, 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
@@ -39,13 +39,14 @@
   friend class JfrThreadSampleClosure;
   friend class ObjectSampleCheckpoint;
   friend class ObjectSampler;
+  friend class RecordStackTrace;
   friend class StackTraceBlobInstaller;
   friend class WriteStackTraceRepository;
 
  private:
   static const u4 TABLE_SIZE = 2053;
   JfrStackTrace* _table[TABLE_SIZE];
-  traceid _next_id;
+  u4 _last_entries;
   u4 _entries;
 
   JfrStackTraceRepository();
@@ -54,20 +55,23 @@
   bool initialize();
   static void destroy();
 
-  size_t write_impl(JfrChunkWriter& cw, bool clear);
-  static void write_metadata(JfrCheckpointWriter& cpw);
-  traceid write(JfrCheckpointWriter& cpw, traceid id, unsigned int hash);
+  bool is_modified() const;
+  static size_t clear();
+  static size_t clear(JfrStackTraceRepository& repo);
   size_t write(JfrChunkWriter& cw, bool clear);
-  size_t clear();
+
+  static const JfrStackTrace* lookup_for_leak_profiler(unsigned int hash, traceid id);
+  static void record_for_leak_profiler(JavaThread* thread, int skip = 0);
+  static void clear_leak_profiler();
 
   traceid add_trace(const JfrStackTrace& stacktrace);
+  static traceid add(JfrStackTraceRepository& repo, const JfrStackTrace& stacktrace);
   static traceid add(const JfrStackTrace& stacktrace);
   traceid record_for(JavaThread* thread, int skip, JfrStackFrame* frames, u4 max_frames);
   const JfrStackTrace* lookup(unsigned int hash, traceid id) const;
 
  public:
   static traceid record(Thread* thread, int skip = 0);
-  static void record_and_cache(JavaThread* thread, int skip = 0);
 };
 
 #endif // SHARE_JFR_RECORDER_STACKTRACE_JFRSTACKTRACEREPOSITORY_HPP
--- a/src/hotspot/share/jfr/support/jfrAllocationTracer.cpp	Wed Apr 21 11:01:25 2021 +0200
+++ b/src/hotspot/share/jfr/support/jfrAllocationTracer.cpp	Wed Apr 21 12:24:28 2021 +0200
@@ -1,5 +1,5 @@
 /*
-* Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
+* Copyright (c) 2016, 2021, 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,16 +28,10 @@
 #include "jfr/support/jfrThreadLocal.hpp"
 #include "runtime/thread.hpp"
 
-JfrAllocationTracer::JfrAllocationTracer(HeapWord* obj, size_t alloc_size, Thread* thread) : _tl(NULL) {
+JfrAllocationTracer::JfrAllocationTracer(HeapWord* obj, size_t alloc_size, Thread* thread) {
   if (LeakProfiler::is_running()) {
     assert(thread->is_Java_thread(), "invariant");
-    _tl = thread->jfr_thread_local();
     LeakProfiler::sample(obj, alloc_size, (JavaThread*)thread);
   }
 }
 
-JfrAllocationTracer::~JfrAllocationTracer() {
-  if (_tl != NULL) {
-    _tl->clear_cached_stack_trace();
-  }
-}
--- a/src/hotspot/share/jfr/support/jfrAllocationTracer.hpp	Wed Apr 21 11:01:25 2021 +0200
+++ b/src/hotspot/share/jfr/support/jfrAllocationTracer.hpp	Wed Apr 21 12:24:28 2021 +0200
@@ -1,5 +1,5 @@
 /*
-* Copyright (c) 2018, Oracle and/or its affiliates. All rights reserved.
+* Copyright (c) 2018, 2021, 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,14 +27,9 @@
 
 #include "memory/allocation.hpp"
 
-class JfrThreadLocal;
-
 class JfrAllocationTracer : public StackObj {
- private:
-  JfrThreadLocal* _tl;
  public:
   JfrAllocationTracer(HeapWord* obj, size_t alloc_size, Thread* thread);
-  ~JfrAllocationTracer();
 };
 
 #endif // SHARE_VM_JFR_SUPPORT_JFRALLOCATIONTRACER_HPP