diff -r ac431929db51 -r a6620d37728b src/hotspot/share/compiler/compileBroker.cpp --- 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()); }