src/hotspot/share/compiler/compileBroker.cpp
changeset 53353 a6620d37728b
parent 52934 8deeb7bba516
child 53406 1ffcf6074569
--- a/src/hotspot/share/compiler/compileBroker.cpp	Tue Jan 15 22:59:33 2019 -0800
+++ b/src/hotspot/share/compiler/compileBroker.cpp	Wed Jan 16 09:48:35 2019 +0100
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1999, 2018, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1999, 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
@@ -2672,6 +2672,7 @@
 //       preventing concurrently printing threads from stalling a long time.
 void CompileBroker::print_heapinfo(outputStream* out, const char* function, const char* granularity) {
   TimeStamp ts_total;
+  TimeStamp ts_global;
   TimeStamp ts;
 
   bool allFun = !strcmp(function, "all");
@@ -2701,35 +2702,43 @@
   }
 
   // We hold the CodeHeapStateAnalytics_lock all the time, from here until we leave this function.
-  // That helps us getting a consistent view on the CodeHeap, at least for the "all" function.
+  // That prevents another thread from destroying our view on the CodeHeap.
   // When we request individual parts of the analysis via the jcmd interface, it is possible
   // that in between another thread (another jcmd user or the vm running into CodeCache OOM)
   // updated the aggregated data. That's a tolerable tradeoff because we can't hold a lock
   // across user interaction.
+  // Acquire this lock before acquiring the CodeCache_lock.
+  // CodeHeapStateAnalytics_lock could be held by a concurrent thread for a long time,
+  // leading to an unnecessarily long hold time of the CodeCache_lock.
   ts.update(); // record starting point
   MutexLockerEx mu1(CodeHeapStateAnalytics_lock, Mutex::_no_safepoint_check_flag);
-  out->cr();
-  out->print_cr("__ CodeHeapStateAnalytics lock wait took %10.3f seconds _________", ts.seconds());
-  out->cr();
+  out->print_cr("\n__ CodeHeapStateAnalytics lock wait took %10.3f seconds _________\n", ts.seconds());
+
+  // If we serve an "allFun" call, it is beneficial to hold the CodeCache_lock
+  // for the entire duration of aggregation and printing. That makes sure
+  // we see a consistent picture and do not run into issues caused by
+  // the CodeHeap being altered concurrently.
+  Monitor* global_lock   = allFun ? CodeCache_lock : NULL;
+  Monitor* function_lock = allFun ? NULL : CodeCache_lock;
+  ts_global.update(); // record starting point
+  MutexLockerEx mu2(global_lock, Mutex::_no_safepoint_check_flag);
+  if (global_lock != NULL) {
+    out->print_cr("\n__ CodeCache (global) lock wait took %10.3f seconds _________\n", ts_global.seconds());
+    ts_global.update(); // record starting point
+  }
 
   if (aggregate) {
-    // It is sufficient to hold the CodeCache_lock only for the aggregate step.
-    // All other functions operate on aggregated data - except MethodNames, but that should be safe.
-    // The separate CodeHeapStateAnalytics_lock protects the printing functions against
-    // concurrent aggregate steps. Acquire this lock before acquiring the CodeCache_lock.
-    // CodeHeapStateAnalytics_lock could be held by a concurrent thread for a long time,
-    // leading to an unnecessarily long hold time of the CodeCache_lock.
     ts.update(); // record starting point
-    MutexLockerEx mu2(CodeCache_lock, Mutex::_no_safepoint_check_flag);
-    out->cr();
-    out->print_cr("__ CodeCache lock wait took %10.3f seconds _________", ts.seconds());
-    out->cr();
+    MutexLockerEx mu3(function_lock, Mutex::_no_safepoint_check_flag);
+    if (function_lock != NULL) {
+      out->print_cr("\n__ CodeCache (function) lock wait took %10.3f seconds _________\n", ts.seconds());
+    }
 
     ts.update(); // record starting point
     CodeCache::aggregate(out, granularity);
-    out->cr();
-    out->print_cr("__ CodeCache lock hold took %10.3f seconds _________", ts.seconds());
-    out->cr();
+    if (function_lock != NULL) {
+      out->print_cr("\n__ CodeCache (function) lock hold took %10.3f seconds _________\n", ts.seconds());
+    }
   }
 
   if (usedSpace) CodeCache::print_usedSpace(out);
@@ -2737,10 +2746,16 @@
   if (methodCount) CodeCache::print_count(out);
   if (methodSpace) CodeCache::print_space(out);
   if (methodAge) CodeCache::print_age(out);
-  if (methodNames) CodeCache::print_names(out);
+  if (methodNames) {
+    // print_names() has shown to be sensitive to concurrent CodeHeap modifications.
+    // Therefore, request  the CodeCache_lock before calling...
+    MutexLockerEx mu3(function_lock, Mutex::_no_safepoint_check_flag);
+    CodeCache::print_names(out);
+  }
   if (discard) CodeCache::discard(out);
 
-  out->cr();
-  out->print_cr("__ CodeHeapStateAnalytics total duration %10.3f seconds _________", ts_total.seconds());
-  out->cr();
+  if (global_lock != NULL) {
+    out->print_cr("\n__ CodeCache (global) lock hold took %10.3f seconds _________\n", ts_global.seconds());
+  }
+  out->print_cr("\n__ CodeHeapStateAnalytics total duration %10.3f seconds _________\n", ts_total.seconds());
 }