changeset 59385:1f03c68c9bc4

8238592: JFR: Crash when dumping paths to gc roots on deep heaps Reviewed-by: mgronlun
author egahlin
date Thu, 21 May 2020 04:09:18 +0200
parents 925f7bba93ef
children 83c1cee639d6
files src/hotspot/share/jfr/leakprofiler/chains/dfsClosure.cpp src/hotspot/share/jfr/leakprofiler/chains/dfsClosure.hpp src/hotspot/share/jfr/leakprofiler/utilities/unifiedOopRef.inline.hpp
diffstat 3 files changed, 38 insertions(+), 61 deletions(-) [+]
line wrap: on
line diff
--- a/src/hotspot/share/jfr/leakprofiler/chains/dfsClosure.cpp	Thu May 21 03:53:18 2020 +0200
+++ b/src/hotspot/share/jfr/leakprofiler/chains/dfsClosure.cpp	Thu May 21 04:09:18 2020 +0200
@@ -37,26 +37,8 @@
 #include "oops/oop.inline.hpp"
 #include "utilities/align.hpp"
 
-// max dfs depth should not exceed size of stack
-static const size_t max_dfs_depth = 5000;
-
-EdgeStore* DFSClosure::_edge_store = NULL;
-BitSet* DFSClosure::_mark_bits = NULL;
-const Edge* DFSClosure::_start_edge = NULL;
-size_t DFSClosure::_max_depth = max_dfs_depth;
-bool DFSClosure::_ignore_root_set = false;
-
-DFSClosure::DFSClosure() :
-  _parent(NULL),
-  _reference(UnifiedOopRef::encode_null()),
-  _depth(0) {
-}
-
-DFSClosure::DFSClosure(DFSClosure* parent, size_t depth) :
-  _parent(parent),
-  _reference(UnifiedOopRef::encode_null()),
-  _depth(depth) {
-}
+ // max dfs depth should not exceed size of stack
+static const size_t max_dfs_depth = 4000;
 
 void DFSClosure::find_leaks_from_edge(EdgeStore* edge_store,
                                       BitSet* mark_bits,
@@ -65,14 +47,8 @@
   assert(mark_bits != NULL," invariant");
   assert(start_edge != NULL, "invariant");
 
-  _edge_store = edge_store;
-  _mark_bits = mark_bits;
-  _start_edge = start_edge;
-  _ignore_root_set = false;
-  assert(_max_depth == max_dfs_depth, "invariant");
-
   // Depth-first search, starting from a BFS egde
-  DFSClosure dfs;
+  DFSClosure dfs(edge_store, mark_bits, start_edge);
   start_edge->pointee()->oop_iterate(&dfs);
 }
 
@@ -81,42 +57,45 @@
   assert(edge_store != NULL, "invariant");
   assert(mark_bits != NULL, "invariant");
 
-  _edge_store = edge_store;
-  _mark_bits = mark_bits;
-  _start_edge = NULL;
-
   // Mark root set, to avoid going sideways
-  _max_depth = 1;
-  _ignore_root_set = false;
-  DFSClosure dfs;
+  DFSClosure dfs(edge_store, mark_bits, NULL);
+  dfs._max_depth = 1;
   RootSetClosure<DFSClosure> rs(&dfs);
   rs.process();
 
   // Depth-first search
-  _max_depth = max_dfs_depth;
-  _ignore_root_set = true;
-  assert(_start_edge == NULL, "invariant");
+  dfs._max_depth = max_dfs_depth;
+  dfs._ignore_root_set = true;
   rs.process();
 }
 
+DFSClosure::DFSClosure(EdgeStore* edge_store, BitSet* mark_bits, const Edge* start_edge)
+  :_edge_store(edge_store), _mark_bits(mark_bits), _start_edge(start_edge),
+  _max_depth(max_dfs_depth), _depth(0), _ignore_root_set(false) {
+  _reference_stack = NEW_C_HEAP_ARRAY(UnifiedOopRef, max_dfs_depth, mtTracing);
+}
+
+DFSClosure::~DFSClosure() {
+  FREE_C_HEAP_ARRAY(UnifiedOopRef, _reference_stack);
+}
+
 void DFSClosure::closure_impl(UnifiedOopRef reference, const oop pointee) {
   assert(pointee != NULL, "invariant");
   assert(!reference.is_null(), "invariant");
 
   if (GranularTimer::is_finished()) {
-     return;
+    return;
   }
   if (_depth == 0 && _ignore_root_set) {
     // Root set is already marked, but we want
     // to continue, so skip is_marked check.
     assert(_mark_bits->is_marked(pointee), "invariant");
-  } else {
+  }  else {
     if (_mark_bits->is_marked(pointee)) {
       return;
     }
   }
-
-  _reference = reference;
+  _reference_stack[_depth] = reference;
   _mark_bits->mark_obj(pointee);
   assert(_mark_bits->is_marked(pointee), "invariant");
 
@@ -127,8 +106,10 @@
 
   assert(_max_depth >= 1, "invariant");
   if (_depth < _max_depth - 1) {
-    DFSClosure next_level(this, _depth + 1);
-    pointee->oop_iterate(&next_level);
+    _depth++;
+    pointee->oop_iterate(this);
+    assert(_depth > 0, "invariant");
+    _depth--;
   }
 }
 
@@ -140,11 +121,10 @@
   size_t idx = 0;
 
   // aggregate from depth-first search
-  const DFSClosure* c = this;
-  while (c != NULL) {
+  for (size_t i = 0; i <= _depth; i++) {
     const size_t next = idx + 1;
-    chain[idx++] = Edge(&chain[next], c->reference());
-    c = c->parent();
+    const size_t depth = _depth - i;
+    chain[idx++] = Edge(&chain[next], _reference_stack[depth]);
   }
   assert(_depth + 1 == idx, "invariant");
   assert(array_length == idx + 1, "invariant");
--- a/src/hotspot/share/jfr/leakprofiler/chains/dfsClosure.hpp	Thu May 21 03:53:18 2020 +0200
+++ b/src/hotspot/share/jfr/leakprofiler/chains/dfsClosure.hpp	Thu May 21 04:09:18 2020 +0200
@@ -36,24 +36,20 @@
 // Class responsible for iterating the heap depth-first
 class DFSClosure : public BasicOopIterateClosure {
  private:
-  static EdgeStore* _edge_store;
-  static BitSet*    _mark_bits;
-  static const Edge*_start_edge;
-  static size_t _max_depth;
-  static bool _ignore_root_set;
-  DFSClosure* _parent;
-  UnifiedOopRef _reference;
+  EdgeStore* _edge_store;
+  BitSet* _mark_bits;
+  const Edge*_start_edge;
+  size_t _max_depth;
   size_t _depth;
+  bool _ignore_root_set;
+  UnifiedOopRef* _reference_stack;
+
+  DFSClosure(EdgeStore* edge_store, BitSet* mark_bits, const Edge* start_edge);
+  ~DFSClosure();
 
   void add_chain();
   void closure_impl(UnifiedOopRef reference, const oop pointee);
 
-  DFSClosure* parent() const { return _parent; }
-  UnifiedOopRef reference() const { return _reference; }
-
-  DFSClosure(DFSClosure* parent, size_t depth);
-  DFSClosure();
-
  public:
   virtual ReferenceIterationMode reference_iteration_mode() { return DO_FIELDS_EXCEPT_REFERENT; }
   virtual bool should_verify_oops() { return false; }
--- a/src/hotspot/share/jfr/leakprofiler/utilities/unifiedOopRef.inline.hpp	Thu May 21 03:53:18 2020 +0200
+++ b/src/hotspot/share/jfr/leakprofiler/utilities/unifiedOopRef.inline.hpp	Thu May 21 04:09:18 2020 +0200
@@ -85,7 +85,8 @@
 }
 
 inline UnifiedOopRef UnifiedOopRef::encode_null() {
-  return UnifiedOopRef();
+  UnifiedOopRef result = { 0 };
+  return result;
 }
 
 inline oop UnifiedOopRef::dereference() const {