8261520: JDK-8261302 breaks runtime/NMT/CheckForProperDetailStackTrace.java

Reviewed-by: zgu, coleenp

Author: tstuefe

src/hotspot/share/services/allocationSite.hpp

@@ -36,7 +36,6 @@ class AllocationSite {
   const MEMFLAGS         _flag;
  public:
   AllocationSite(const NativeCallStack& stack, MEMFLAGS flag) : _call_stack(stack), _flag(flag) { }
-  int hash() const { return _call_stack.hash(); }
 
   bool equals(const NativeCallStack& stack) const {
     return _call_stack.equals(stack);

src/hotspot/share/services/mallocSiteTable.cpp

@@ -81,7 +81,7 @@ bool MallocSiteTable::initialize() {
   _hash_entry_allocation_site = &entry;
 
   // Add the allocation site to hashtable.
-  int index = hash_to_index(stack.hash());
+  int index = hash_to_index(entry.hash());
   _table[index] = const_cast<MallocSiteHashtableEntry*>(&entry);
 
   return true;
@@ -117,7 +117,8 @@ bool MallocSiteTable::walk(MallocSiteWalker* walker) {
 MallocSite* MallocSiteTable::lookup_or_add(const NativeCallStack& key, size_t* bucket_idx,
   size_t* pos_idx, MEMFLAGS flags) {
   assert(flags != mtNone, "Should have a real memory type");
-  unsigned int index = hash_to_index(key.hash());
+  const unsigned int hash = key.calculate_hash();
+  const unsigned int index = hash_to_index(hash);
   *bucket_idx = (size_t)index;
   *pos_idx = 0;
 
@@ -137,9 +138,11 @@ MallocSite* MallocSiteTable::lookup_or_add(const NativeCallStack& key, size_t* b
 
   MallocSiteHashtableEntry* head = _table[index];
   while (head != NULL && (*pos_idx) <= MAX_BUCKET_LENGTH) {
-    MallocSite* site = head->data();
-    if (site->flag() == flags && site->equals(key)) {
-      return head->data();
+    if (head->hash() == hash) {
+      MallocSite* site = head->data();
+      if (site->flag() == flags && site->equals(key)) {
+        return head->data();
+      }
     }
 
     if (head->next() == NULL && (*pos_idx) < MAX_BUCKET_LENGTH) {

src/hotspot/share/services/mallocSiteTable.hpp

@@ -57,12 +57,13 @@ class MallocSite : public AllocationSite {
 class MallocSiteHashtableEntry : public CHeapObj<mtNMT> {
  private:
   MallocSite                         _malloc_site;
+  const unsigned int                 _hash;
   MallocSiteHashtableEntry* volatile _next;
 
  public:
 
   MallocSiteHashtableEntry(NativeCallStack stack, MEMFLAGS flags):
-    _malloc_site(stack, flags), _next(NULL) {
+    _malloc_site(stack, flags), _hash(stack.calculate_hash()), _next(NULL) {
     assert(flags != mtNone, "Expect a real memory type");
   }
 
@@ -75,6 +76,8 @@ class MallocSiteHashtableEntry : public CHeapObj<mtNMT> {
   // The operation can be failed due to contention from other thread.
   bool atomic_insert(MallocSiteHashtableEntry* entry);
 
+  unsigned int hash() const { return _hash; }
+
   inline const MallocSite* peek() const { return &_malloc_site; }
   inline MallocSite* data()             { return &_malloc_site; }
 

src/hotspot/share/utilities/nativeCallStack.cpp

@@ -30,16 +30,7 @@
 
 const NativeCallStack NativeCallStack::_empty_stack; // Uses default ctor
 
-static unsigned int calculate_hash(address stack[NMT_TrackingStackDepth]) {
-  uintptr_t hash = 0;
-  for (int i = 0; i < NMT_TrackingStackDepth; i++) {
-    hash += (uintptr_t)stack[i];
-  }
-  return hash;
-}
-
-NativeCallStack::NativeCallStack(int toSkip) :
-  _hash_value(0) {
+NativeCallStack::NativeCallStack(int toSkip) {
 
   // We need to skip the NativeCallStack::NativeCallStack frame if a tail call is NOT used
   // to call os::get_native_stack. A tail call is used if _NMT_NOINLINE_ is not defined
@@ -55,7 +46,6 @@ NativeCallStack::NativeCallStack(int toSkip) :
 #endif // Special-case for BSD.
 #endif // Not a tail call.
   os::get_native_stack(_stack, NMT_TrackingStackDepth, toSkip);
-  _hash_value = calculate_hash(_stack);
 }
 
 NativeCallStack::NativeCallStack(address* pc, int frameCount) {
@@ -68,7 +58,6 @@ NativeCallStack::NativeCallStack(address* pc, int frameCount) {
   for (; index < NMT_TrackingStackDepth; index ++) {
     _stack[index] = NULL;
   }
-  _hash_value = calculate_hash(_stack);
 }
 
 // number of stack frames captured

src/hotspot/share/utilities/nativeCallStack.hpp

@@ -56,12 +56,11 @@ class MemTracker;
 class NativeCallStack : public StackObj {
 private:
   address       _stack[NMT_TrackingStackDepth];
-  unsigned int  _hash_value;
   static const NativeCallStack _empty_stack;
 public:
   // Default ctor creates an empty stack.
   // (it may make sense to remove this altogether but its used in a few places).
-  NativeCallStack() : _hash_value(0) {
+  NativeCallStack() {
     memset(_stack, 0, sizeof(_stack));
   }
 
@@ -83,9 +82,6 @@ class NativeCallStack : public StackObj {
   }
 
   inline bool equals(const NativeCallStack& other) const {
-    // compare hash values
-    if (hash() != other.hash()) return false;
-    // compare each frame
     return compare(other) == 0;
   }
 
@@ -94,7 +90,14 @@ class NativeCallStack : public StackObj {
     return _stack[index];
   }
 
-  unsigned int hash() const { return _hash_value; }
+  // Helper; calculates a hash value over the stack frames in this stack
+  unsigned int calculate_hash() const {
+    uintptr_t hash = 0;
+    for (int i = 0; i < NMT_TrackingStackDepth; i++) {
+      hash += (uintptr_t)_stack[i];
+    }
+    return hash;
+  }
 
   void print_on(outputStream* out) const;
   void print_on(outputStream* out, int indent) const;

test/hotspot/jtreg/ProblemList.txt

@@ -85,7 +85,6 @@ gc/metaspace/CompressedClassSpaceSizeInJmapHeap.java 8241293 macosx-x64
 runtime/cds/appcds/jigsaw/modulepath/ModulePathAndCP_JFR.java 8253437 windows-x64
 runtime/cds/DeterministicDump.java 8253495 generic-all
 runtime/jni/terminatedThread/TestTerminatedThread.java 8219652 aix-ppc64
-runtime/NMT/CheckForProperDetailStackTrace.java 8261520 generic-all
 runtime/ReservedStack/ReservedStackTest.java 8231031 generic-all
 containers/docker/TestJFRWithJMX.java 8256417 linux-5.4.17-2011.5.3.el8uek.x86_64