8333994: NMT: call stacks should show source information

tstuefe · tstuefe · commit e1390056c9db · 2024-06-26T08:44:17.000Z
Reviewed-by: jsjolen, gziemski
diff --git a/src/hotspot/share/nmt/memReporter.cpp b/src/hotspot/share/nmt/memReporter.cpp
@@ -337,7 +337,7 @@ int MemDetailReporter::report_malloc_sites() {
       continue;
     }
     const NativeCallStack* stack = malloc_site->call_stack();
-    stack->print_on(out);
+    _stackprinter.print_stack(stack);
     MEMFLAGS flag = malloc_site->flag();
     assert(NMTUtil::flag_is_valid(flag) && flag != mtNone,
       "Must have a valid memory type");
@@ -374,7 +374,7 @@ int MemDetailReporter::report_virtual_memory_allocation_sites()  {
       continue;
     }
     const NativeCallStack* stack = virtual_memory_site->call_stack();
-    stack->print_on(out);
+    _stackprinter.print_stack(stack);
     INDENT_BY(29,
       out->print("(");
       print_total(virtual_memory_site->reserved(), virtual_memory_site->committed());
@@ -428,7 +428,7 @@ void MemDetailReporter::report_virtual_memory_region(const ReservedMemoryRegion*
     out->cr();
   } else {
     out->print_cr(" from");
-    INDENT_BY(4, stack->print_on(out);)
+    INDENT_BY(4, _stackprinter.print_stack(stack);)
   }
 
   if (all_committed) {
@@ -869,7 +869,7 @@ void MemDetailDiffReporter::diff_malloc_site(const NativeCallStack* stack, size_
       return;
   }
 
-  stack->print_on(out);
+  _stackprinter.print_stack(stack);
   INDENT_BY(28,
     out->print("(");
     print_malloc_diff(current_size, current_count, early_size, early_count, flags);
@@ -904,7 +904,7 @@ void MemDetailDiffReporter::diff_virtual_memory_site(const NativeCallStack* stac
     return;
   }
 
-  stack->print_on(out);
+  _stackprinter.print_stack(stack);
   INDENT_BY(28,
     out->print("(mmap: ");
     print_virtual_memory_diff(current_reserved, current_committed, early_reserved, early_committed);
diff --git a/src/hotspot/share/nmt/memReporter.hpp b/src/hotspot/share/nmt/memReporter.hpp
@@ -28,8 +28,10 @@
 #include "memory/metaspace.hpp"
 #include "nmt/mallocTracker.hpp"
 #include "nmt/memBaseline.hpp"
+#include "nmt/nativeCallStackPrinter.hpp"
 #include "nmt/nmtCommon.hpp"
 #include "nmt/virtualMemoryTracker.hpp"
+#include "utilities/nativeCallStack.hpp"
 
 /*
  * Base class that provides helpers
@@ -149,11 +151,11 @@ class MemSummaryReporter : public MemReporterBase {
 class MemDetailReporter : public MemSummaryReporter {
  private:
   MemBaseline&   _baseline;
-
+  NativeCallStackPrinter _stackprinter;
  public:
   MemDetailReporter(MemBaseline& baseline, outputStream* output, size_t scale = default_scale) :
     MemSummaryReporter(baseline, output, scale),
-     _baseline(baseline) { }
+     _baseline(baseline), _stackprinter(output) { }
 
   // Generate detail report.
   // The report contains summary and detail sections.
@@ -229,10 +231,12 @@ class MemSummaryDiffReporter : public MemReporterBase {
  * both baselines have to be detail baseline.
  */
 class MemDetailDiffReporter : public MemSummaryDiffReporter {
+  NativeCallStackPrinter _stackprinter;
  public:
   MemDetailDiffReporter(MemBaseline& early_baseline, MemBaseline& current_baseline,
     outputStream* output, size_t scale = default_scale) :
-    MemSummaryDiffReporter(early_baseline, current_baseline, output, scale) { }
+    MemSummaryDiffReporter(early_baseline, current_baseline, output, scale),
+    _stackprinter(output) { }
 
   // Generate detail comparison report
   virtual void report_diff();
diff --git a/src/hotspot/share/nmt/nativeCallStackPrinter.cpp b/src/hotspot/share/nmt/nativeCallStackPrinter.cpp
@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) 2024, Red Hat, Inc. All rights reserved.
+ * Copyright (c) 2024, 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.
+ *
+ */
+
+#include "precompiled.hpp"
+#include "logging/log.hpp"
+#include "nmt/nativeCallStackPrinter.hpp"
+#include "utilities/globalDefinitions.hpp"
+#include "utilities/nativeCallStack.hpp"
+#include "utilities/ostream.hpp"
+
+NativeCallStackPrinter::NativeCallStackPrinter(outputStream* out) :
+    _text_storage(mtNMT, Arena::Tag::tag_other, 128 * K), _out(out)
+{}
+
+void NativeCallStackPrinter::print_stack(const NativeCallStack* stack) const {
+  for (int i = 0; i < NMT_TrackingStackDepth; i++) {
+    const address pc = stack->get_frame(i);
+    if (pc == nullptr) {
+      break;
+    }
+    bool created = false;
+    const char** cached_frame_text = _cache.put_if_absent(pc, &created);
+    if (created) {
+      stringStream ss(4 * K);
+      stack->print_frame(&ss, pc);
+      const size_t len = ss.size();
+      char* store = NEW_ARENA_ARRAY(&_text_storage, char, len + 1);
+      memcpy(store, ss.base(), len + 1);
+      (*cached_frame_text) = store;
+    }
+    _out->print_raw_cr(*cached_frame_text);
+  }
+}
diff --git a/src/hotspot/share/nmt/nativeCallStackPrinter.hpp b/src/hotspot/share/nmt/nativeCallStackPrinter.hpp
@@ -0,0 +1,51 @@
+/*
+ * Copyright (c) 2024, Red Hat, Inc. All rights reserved.
+ * Copyright (c) 2024, 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_NMT_NATIVECALLSTACKPRINTER_HPP
+#define SHARE_NMT_NATIVECALLSTACKPRINTER_HPP
+
+#include "memory/arena.hpp"
+#include "nmt/memflags.hpp"
+#include "utilities/globalDefinitions.hpp"
+#include "utilities/resourceHash.hpp"
+
+class outputStream;
+class NativeCallStack;
+
+// This is a text cache for NativeCallStack frames by PC. When printing tons of
+// NativeCallStack instances (e.g. during NMT detail reports), printing through
+// this printer speeds up frame description resolution by quite a bit.
+class NativeCallStackPrinter {
+  // Cache-related data are mutable to be able to use NativeCallStackPrinter as
+  // inline member in classes with const printing methods.
+  mutable Arena _text_storage;
+  mutable ResourceHashtable<address, const char*, 293, AnyObj::C_HEAP, mtNMT> _cache;
+  outputStream* const _out;
+public:
+  NativeCallStackPrinter(outputStream* out);
+  void print_stack(const NativeCallStack* stack) const;
+};
+
+#endif // SHARE_NMT_NATIVECALLSTACKPRINTER_HPP
diff --git a/src/hotspot/share/nmt/virtualMemoryTracker.cpp b/src/hotspot/share/nmt/virtualMemoryTracker.cpp
@@ -26,6 +26,7 @@
 #include "memory/metaspaceStats.hpp"
 #include "memory/metaspaceUtils.hpp"
 #include "nmt/memTracker.hpp"
+#include "nmt/nativeCallStackPrinter.hpp"
 #include "nmt/threadStackTracker.hpp"
 #include "nmt/virtualMemoryTracker.hpp"
 #include "runtime/os.hpp"
@@ -679,16 +680,17 @@ class PrintRegionWalker : public VirtualMemoryWalker {
 private:
   const address               _p;
   outputStream*               _st;
+  NativeCallStackPrinter      _stackprinter;
 public:
   PrintRegionWalker(const void* p, outputStream* st) :
-    _p((address)p), _st(st) { }
+    _p((address)p), _st(st), _stackprinter(st) { }
 
   bool do_allocation_site(const ReservedMemoryRegion* rgn) {
     if (rgn->contain_address(_p)) {
       _st->print_cr(PTR_FORMAT " in mmap'd memory region [" PTR_FORMAT " - " PTR_FORMAT "], tag %s",
         p2i(_p), p2i(rgn->base()), p2i(rgn->base() + rgn->size()), NMTUtil::flag_to_enum_name(rgn->flag()));
       if (MemTracker::tracking_level() == NMT_detail) {
-        rgn->call_stack()->print_on(_st);
+        _stackprinter.print_stack(rgn->call_stack());
         _st->cr();
       }
       return false;
diff --git a/src/hotspot/share/utilities/nativeCallStack.cpp b/src/hotspot/share/utilities/nativeCallStack.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2014, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2014, 2024, 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
@@ -72,44 +72,48 @@ int NativeCallStack::frames() const {
 }
 
 // Decode and print this call path
-void NativeCallStack::print_on(outputStream* out) const {
-  DEBUG_ONLY(assert_not_fake();)
-  address pc;
+
+void NativeCallStack::print_frame(outputStream* out, address pc) const {
   char    buf[1024];
   int     offset;
-  if (is_empty()) {
-    out->print("[BOOTSTRAP]");
-  } else {
-    for (int frame = 0; frame < NMT_TrackingStackDepth; frame ++) {
-      pc = get_frame(frame);
-      if (pc == nullptr) break;
-      out->print("[" PTR_FORMAT "]", p2i(pc));
-      // Print function and library; shorten library name to just its last component
-      // for brevity, and omit it completely for libjvm.so
-      bool function_printed = false;
-      if (os::dll_address_to_function_name(pc, buf, sizeof(buf), &offset)) {
-        out->print("%s+0x%x", buf, offset);
-        function_printed = true;
-      }
-      if ((!function_printed || !os::address_is_in_vm(pc)) &&
-          os::dll_address_to_library_name(pc, buf, sizeof(buf), &offset)) {
-        const char* libname = strrchr(buf, os::file_separator()[0]);
-        if (libname != nullptr) {
-          libname++;
-        } else {
-          libname = buf;
-        }
-        out->print(" in %s", libname);
-        if (!function_printed) {
-          out->print("+0x%x", offset);
-        }
+  int     line;
+  const bool pc_in_VM = os::address_is_in_vm(pc);
+  out->print("[" PTR_FORMAT "]", p2i(pc));
+  // Print function and library; shorten library name to just its last component
+  // for brevity, and omit it completely for libjvm.so
+  bool function_printed = false;
+  if (os::dll_address_to_function_name(pc, buf, sizeof(buf), &offset)) {
+    out->print("%s+0x%x", buf, offset);
+    function_printed = true;
+    if (Decoder::get_source_info(pc, buf, sizeof(buf), &line, false)) {
+      // For intra-vm functions, we omit the full path
+      const char* s = buf;
+      if (pc_in_VM) {
+        s = strrchr(s, os::file_separator()[0]);
+        s = (s != nullptr) ? s + 1 : buf;
       }
-
-      // Note: we deliberately omit printing source information here. NativeCallStack::print_on()
-      // can be called thousands of times as part of NMT detail reporting, and source printing
-      // can slow down reporting by a factor of 5 or more depending on platform (see JDK-8296931).
-
-      out->cr();
+      out->print("   (%s:%d)", s, line);
+    }
+  }
+  if ((!function_printed || !pc_in_VM) &&
+      os::dll_address_to_library_name(pc, buf, sizeof(buf), &offset)) {
+    const char* libname = strrchr(buf, os::file_separator()[0]);
+    if (libname != nullptr) {
+      libname++;
+    } else {
+      libname = buf;
+    }
+    out->print(" in %s", libname);
+    if (!function_printed) {
+      out->print("+0x%x", offset);
     }
   }
 }
+
+void NativeCallStack::print_on(outputStream* out) const {
+  DEBUG_ONLY(assert_not_fake();)
+  for (int i = 0; i < NMT_TrackingStackDepth && _stack[i] != nullptr; i++) {
+    print_frame(out, _stack[i]);
+  }
+  out->cr();
+}
diff --git a/src/hotspot/share/utilities/nativeCallStack.hpp b/src/hotspot/share/utilities/nativeCallStack.hpp
@@ -28,6 +28,7 @@
 #include "memory/allocation.hpp"
 #include "nmt/nmtCommon.hpp"
 #include "utilities/ostream.hpp"
+#include "utilities/resourceHash.hpp"
 
 /*
  * This class represents a native call path (does not include Java frame)
@@ -123,6 +124,7 @@ class NativeCallStack : public StackObj {
     return (unsigned int)hash;
   }
 
+  void print_frame(outputStream* out, address pc) const;
   void print_on(outputStream* out) const;
 };
 
diff --git a/test/hotspot/jtreg/runtime/NMT/CheckForProperDetailStackTrace.java b/test/hotspot/jtreg/runtime/NMT/CheckForProperDetailStackTrace.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2016, 2024, 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
@@ -58,6 +58,8 @@ public class CheckForProperDetailStackTrace {
     private static final Path SRC_DIR = Paths.get(TEST_SRC, "src");
     private static final Path MODS_DIR = Paths.get(TEST_CLASSES, "mods");
 
+    private static final boolean expectSourceInformation = Platform.isLinux() || Platform.isWindows();
+
     /* The stack trace we look for by default. Note that :: has been replaced by .*
        to make sure it matches even if the symbol is not unmangled.
     */
@@ -121,29 +123,27 @@ public static void main(String args[]) throws Exception {
             // It's ok for ARM not to have symbols, because it does not support NMT detail
             // when targeting thumb2. It's also ok for Windows not to have symbols, because
             // they are only available if the symbols file is included with the build.
-            if (Platform.isWindows() || Platform.isARM()) {
-                return; // we are done
+            if (!Platform.isWindows() && !Platform.isARM()) {
+                output.reportDiagnosticSummary();
+                throw new RuntimeException("Expected symbol missing from output: " + expectedSymbol);
             }
-            output.reportDiagnosticSummary();
-            throw new RuntimeException("Expected symbol missing from output: " + expectedSymbol);
         }
 
         // Make sure the expected NMT detail stack trace is found
         System.out.println("Looking for a stack matching:");
-        if (okToHaveAllocateHeap) {
-            System.out.print(stackTraceAllocateHeap);
-            if (stackTraceMatches(stackTraceAllocateHeap, output)) {
-                return;
-            }
-        } else {
-            System.out.print(stackTraceDefault);
-            if (stackTraceMatches(stackTraceDefault, output)) {
-                return;
+        String toMatch = okToHaveAllocateHeap ? stackTraceAllocateHeap : stackTraceDefault;
+        if (!stackTraceMatches(toMatch, output)) {
+            output.reportDiagnosticSummary();
+            throw new RuntimeException("Expected stack trace missing from output");
+        }
+
+        System.out.println("Looking for source information:");
+        if (expectSourceInformation) {
+            if (!stackTraceMatches(".*moduleEntry.cpp.*", output)) {
+                output.reportDiagnosticSummary();
+                throw new RuntimeException("Expected source information missing from output");
             }
         }
-        // Failed to match so dump all the output
-        output.reportDiagnosticSummary();
-        throw new RuntimeException("Expected stack trace missing from output");
     }
 
     public static boolean stackTraceMatches(String stackTrace, OutputAnalyzer output) {
