simplifications JEP-349-branch
authormgronlun
Wed, 28 Aug 2019 12:03:58 +0200
branchJEP-349-branch
changeset 57902 f6502e486572
parent 57887 a9cc3698a55c
child 57920 de358b994ad9
simplifications
src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleCheckpoint.cpp
src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.cpp
src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.hpp
src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceId.inline.hpp
src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdMacros.hpp
src/hotspot/share/jfr/recorder/stacktrace/jfrStackTraceRepository.cpp
src/hotspot/share/jfr/recorder/stacktrace/jfrStackTraceRepository.hpp
--- a/src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleCheckpoint.cpp	Tue Aug 27 12:36:05 2019 +0200
+++ b/src/hotspot/share/jfr/leakprofiler/checkpoint/objectSampleCheckpoint.cpp	Wed Aug 28 12:03:58 2019 +0200
@@ -418,7 +418,7 @@
   assert(stack_trace != NULL, "invariant");
   DEBUG_ONLY(validate_stack_trace(sample, stack_trace));
   JfrStackTrace* const sample_trace = const_cast<JfrStackTrace*>(sample->stack_trace());
-  if (sample_trace != NULL) {
+  if (sample_trace != NULL && sample_trace->id() != stack_trace->id()) {
     *sample_trace = *stack_trace; // copy
   } else {
     sample->set_stack_trace(new JfrStackTrace(stack_trace->id(), *stack_trace, NULL)); // new
--- a/src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.cpp	Tue Aug 27 12:36:05 2019 +0200
+++ b/src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.cpp	Wed Aug 28 12:03:58 2019 +0200
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2019, 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,53 +117,32 @@
   return tl->thread_id();
 }
 
-// Populates the thread local stack frames, but does not add them
-// to the stacktrace repository (...yet, see stacktrace_id() below)
-//
-void ObjectSampler::fill_stacktrace(JfrStackTrace* stacktrace, JavaThread* thread) {
-  assert(stacktrace != NULL, "invariant");
+static void record_stacktrace(JavaThread* thread) {
   assert(thread != NULL, "invariant");
   if (JfrEventSetting::has_stacktrace(EventOldObjectSample::eventId)) {
-    JfrStackTraceRepository::fill_stacktrace_for(thread, stacktrace, 0);
+    JfrStackTraceRepository::record_and_cache(thread);
   }
 }
 
-// We were successful in acquiring the try lock and have been selected for adding a sample.
-// Go ahead with installing our previously taken stacktrace into the stacktrace repository.
-//
-traceid ObjectSampler::stacktrace_id(const JfrStackTrace* stacktrace, JavaThread* thread) {
-  assert(stacktrace != NULL, "invariant");
-  assert(stacktrace->hash() != 0, "invariant");
-  const traceid stacktrace_id = JfrStackTraceRepository::add(stacktrace, thread);
-  thread->jfr_thread_local()->set_cached_stack_trace_id(stacktrace_id, stacktrace->hash());
-  return stacktrace_id;
-}
-
 void ObjectSampler::sample(HeapWord* obj, size_t allocated, JavaThread* thread) {
   assert(thread != NULL, "invariant");
   assert(is_created(), "invariant");
-
   const traceid thread_id = get_thread_id(thread);
   if (thread_id == 0) {
     return;
   }
-
-  const JfrThreadLocal* const tl = thread->jfr_thread_local();
-  JfrStackTrace stacktrace(tl->stackframes(), tl->stackdepth());
-  fill_stacktrace(&stacktrace, thread);
-
+  record_stacktrace(thread);
   // try enter critical section
   JfrTryLock tryLock(&_lock);
   if (!tryLock.has_lock()) {
     log_trace(jfr, oldobject, sampling)("Skipping old object sample due to lock contention");
     return;
   }
-
-  instance().add(obj, allocated, thread_id, &stacktrace, thread);
+  instance().add(obj, allocated, thread_id, thread);
 }
 
-void ObjectSampler::add(HeapWord* obj, size_t allocated, traceid thread_id, JfrStackTrace* stacktrace, JavaThread* thread) {
-  assert(stacktrace != NULL, "invariant");
+void ObjectSampler::add(HeapWord* obj, size_t allocated, traceid thread_id, JavaThread* thread) {
+  assert(obj != NULL, "invariant");
   assert(thread_id != 0, "invariant");
   assert(thread != NULL, "invariant");
   assert(thread->jfr_thread_local()->has_thread_checkpoint(), "invariant");
@@ -190,11 +169,13 @@
 
   assert(sample != NULL, "invariant");
   sample->set_thread_id(thread_id);
-  sample->set_thread_checkpoint(thread->jfr_thread_local()->thread_checkpoint());
+
+  const JfrThreadLocal* const tl = thread->jfr_thread_local();
+  sample->set_thread_checkpoint(tl->thread_checkpoint());
 
-  const unsigned int stacktrace_hash = stacktrace->hash();
+  const unsigned int stacktrace_hash = tl->cached_stack_trace_hash();
   if (stacktrace_hash != 0) {
-    sample->set_stack_trace_id(stacktrace_id(stacktrace, thread));
+    sample->set_stack_trace_id(tl->cached_stack_trace_id());
     sample->set_stack_trace_hash(stacktrace_hash);
   }
 
--- a/src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.hpp	Tue Aug 27 12:36:05 2019 +0200
+++ b/src/hotspot/share/jfr/leakprofiler/sampling/objectSampler.hpp	Wed Aug 28 12:03:58 2019 +0200
@@ -32,7 +32,6 @@
 
 class BoolObjectClosure;
 class JavaThread;
-class JfrStackTrace;
 class OopClosure;
 class ObjectSample;
 class SampleList;
@@ -61,13 +60,9 @@
   static bool is_created();
   static void destroy();
 
-  // Stacktrace
-  static void fill_stacktrace(JfrStackTrace* stacktrace, JavaThread* thread);
-  traceid stacktrace_id(const JfrStackTrace* stacktrace, JavaThread* thread);
-
   // Sampling
   static void sample(HeapWord* object, size_t size, JavaThread* thread);
-  void add(HeapWord* object, size_t size, traceid thread_id, JfrStackTrace* stacktrace, JavaThread* thread);
+  void add(HeapWord* object, size_t size, traceid thread_id, JavaThread* thread);
   void scavenge();
   void remove_dead(ObjectSample* sample);
 
--- a/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceId.inline.hpp	Tue Aug 27 12:36:05 2019 +0200
+++ b/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceId.inline.hpp	Wed Aug 28 12:03:58 2019 +0200
@@ -29,8 +29,10 @@
 #include "classfile/moduleEntry.hpp"
 #include "classfile/packageEntry.hpp"
 #include "jfr/recorder/checkpoint/types/traceid/jfrTraceId.hpp"
+#include "jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp"
 #include "jfr/recorder/checkpoint/types/traceid/jfrTraceIdEpoch.hpp"
 #include "jfr/recorder/checkpoint/types/traceid/jfrTraceIdMacros.hpp"
+#include "jfr/support/jfrKlassExtension.hpp"
 #include "oops/arrayKlass.hpp"
 #include "oops/klass.hpp"
 #include "oops/instanceKlass.hpp"
--- a/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdMacros.hpp	Tue Aug 27 12:36:05 2019 +0200
+++ b/src/hotspot/share/jfr/recorder/checkpoint/types/traceid/jfrTraceIdMacros.hpp	Wed Aug 28 12:03:58 2019 +0200
@@ -25,11 +25,6 @@
 #ifndef SHARE_JFR_RECORDER_CHECKPOINT_TYPES_TRACEID_JFRTRACEIDMACROS_HPP
 #define SHARE_JFR_RECORDER_CHECKPOINT_TYPES_TRACEID_JFRTRACEIDMACROS_HPP
 
-#include "jfr/recorder/checkpoint/types/traceid/jfrTraceIdBits.inline.hpp"
-#include "jfr/recorder/checkpoint/types/traceid/jfrTraceIdEpoch.hpp"
-#include "jfr/support/jfrKlassExtension.hpp"
-#include "utilities/globalDefinitions.hpp"
-
 /**
  *
  * If a traceid is used, depending on epoch, either the first or the second bit is tagged.
--- a/src/hotspot/share/jfr/recorder/stacktrace/jfrStackTraceRepository.cpp	Tue Aug 27 12:36:05 2019 +0200
+++ b/src/hotspot/share/jfr/recorder/stacktrace/jfrStackTraceRepository.cpp	Wed Aug 28 12:03:58 2019 +0200
@@ -27,6 +27,7 @@
 #include "jfr/recorder/checkpoint/jfrCheckpointWriter.hpp"
 #include "jfr/recorder/repository/jfrChunkWriter.hpp"
 #include "jfr/recorder/stacktrace/jfrStackTraceRepository.hpp"
+#include "jfr/support/jfrThreadLocal.hpp"
 #include "runtime/mutexLocker.hpp"
 
 static JfrStackTraceRepository* _instance = NULL;
@@ -158,7 +159,7 @@
   }
   assert(frames != NULL, "invariant");
   assert(tl->stackframes() == frames, "invariant");
-  return instance().record_for((JavaThread*)thread, skip,frames, tl->stackdepth());
+  return instance().record_for((JavaThread*)thread, skip, frames, tl->stackdepth());
 }
 
 traceid JfrStackTraceRepository::record_for(JavaThread* thread, int skip, JfrStackFrame *frames, u4 max_frames) {
@@ -166,13 +167,6 @@
   return stacktrace.record_safe(thread, skip) ? add(stacktrace) : 0;
 }
 
-traceid JfrStackTraceRepository::add(const JfrStackTrace* stacktrace, JavaThread* thread) {
-  assert(stacktrace != NULL, "invariant");
-  assert(thread != NULL, "invariant");
-  assert(stacktrace->hash() != 0, "invariant");
-  return add(*stacktrace);
-}
-
 traceid JfrStackTraceRepository::add(const JfrStackTrace& stacktrace) {
   traceid tid = instance().add_trace(stacktrace);
   if (tid == 0) {
@@ -183,6 +177,20 @@
   return tid;
 }
 
+traceid JfrStackTraceRepository::record_and_cache(JavaThread* thread, int skip /* 0 */) {
+  JfrThreadLocal* const tl = thread->jfr_thread_local();
+  assert(tl != NULL, "invariant");
+  if (tl->has_cached_stack_trace()) {
+    return tl->cached_stack_trace_id();
+  }
+  JfrStackTrace stacktrace(tl->stackframes(), tl->stackdepth());
+  stacktrace.record_safe(thread, skip);
+  assert(stacktrace.hash() != 0, "invariant");
+  const traceid id = instance().add(stacktrace);
+  tl->set_cached_stack_trace_id(id, stacktrace.hash());
+  return id;
+}
+
 traceid JfrStackTraceRepository::add_trace(const JfrStackTrace& stacktrace) {
   MutexLocker lock(JfrStacktrace_lock, Mutex::_no_safepoint_check_flag);
   const size_t index = stacktrace._hash % TABLE_SIZE;
@@ -217,17 +225,3 @@
   assert(trace->id() == id, "invariant");
   return trace;
 }
-
-bool JfrStackTraceRepository::fill_stacktrace_for(JavaThread* thread, JfrStackTrace* stacktrace, int skip) {
-  assert(thread == Thread::current(), "invariant");
-  assert(stacktrace != NULL, "invariant");
-  JfrThreadLocal* const tl = thread->jfr_thread_local();
-  assert(tl != NULL, "invariant");
-  const unsigned int cached_stacktrace_hash = tl->cached_stack_trace_hash();
-  if (cached_stacktrace_hash != 0) {
-    stacktrace->set_hash(cached_stacktrace_hash);
-    return true;
-  }
-  return stacktrace->record_safe(thread, skip);
-}
-
--- a/src/hotspot/share/jfr/recorder/stacktrace/jfrStackTraceRepository.hpp	Tue Aug 27 12:36:05 2019 +0200
+++ b/src/hotspot/share/jfr/recorder/stacktrace/jfrStackTraceRepository.hpp	Wed Aug 28 12:03:58 2019 +0200
@@ -37,6 +37,7 @@
   friend class FlushStackTraceRepository;
   friend class JfrRecorder;
   friend class JfrRecorderService;
+  friend class JfrThreadSampleClosure;
   friend class ObjectSampleCheckpoint;
   friend class ObjectSampler;
   friend class StackTraceInstall;
@@ -64,14 +65,13 @@
   size_t clear();
 
   traceid add_trace(const JfrStackTrace& stacktrace);
-  static traceid add(const JfrStackTrace* stacktrace, JavaThread* thread);
+  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;
-  static bool fill_stacktrace_for(JavaThread* thread, JfrStackTrace* stacktrace, int skip);
 
  public:
-  static traceid add(const JfrStackTrace& stacktrace);
   static traceid record(Thread* thread, int skip = 0);
+  static traceid record_and_cache(JavaThread* thread, int skip = 0);
 };
 
 #endif // SHARE_JFR_RECORDER_STACKTRACE_JFRSTACKTRACEREPOSITORY_HPP