Hi Daniil, I've not finished my review but it looks good in general.
A couple of quick comments. https://cr.openjdk.java.net/~dtitov/8170299/webrev.01/src/hotspot/share/runtime/notificationThread.hpp.html I wonder if this function is also needed: static bool is_notification_thread(Thread* thread); https://cr.openjdk.java.net/~dtitov/8170299/webrev.01/src/hotspot/share/runtime/notificationThread.cpp.html I wonder why this include statement is missed: #include "runtime/mutexLocker.hpp" Also, these have to be correctly ordred: 29 #include "runtime/notificationThread.hpp" 30 #include "services/lowMemoryDetector.hpp" 31 #include "services/gcNotifier.hpp" 32 #include "services/diagnosticArgument.hpp" 33 #include "services/diagnosticFramework.hpp" Thanks, Serguei On 7/3/19 8:04 PM, Daniil Titov wrote:
Please review the change the fixes the problem with the debugger not stopping in the low memory notification code. The problem here is that the ServiceThread that calls these MXBean listeners is hidden from the external view that prevents the debugger from stopping in it. The fix introduces new NotificationThread that is visible to the external view and offloads the ServiceThread from sending low memory and other notifications that could result in Java calls ( GC and diagnostic commands notifications) by moving these activities in this new NotificationThread. Testing: Mach5 tier1,tier2 and tier3 tests succeeded. Webrev: https://cr.openjdk.java.net/~dtitov/8170299/webrev.01/ Bug: https://bugs.openjdk.java.net/browse/JDK-8170299 Thanks! --Daniil