8170299: Debugger does not stop inside the low memory notifications code
serguei.spitsyn at oracle.com
serguei.spitsyn at oracle.com
Tue Jul 9 16:02:11 UTC 2019
Hi Daniil,
Missed it, sorry.
Thanks,
Serguei
On 7/9/19 08:37, Daniil Titov wrote:
> Hi Serguei,
>
> I tried to answer your question regarding "runtime/mutexLocker.hpp" in my reply.
>
>> The list of Include statements doesn't contain "#include "runtime/mutexLocker.hpp" since this include file is already included by runtime/interfaceSupport.inline.hpp that is in this list.
> File "src/hotspot/share/runtime/notificationThread.cpp" includes "runtime/interfaceSupport.inline.hpp" and the header file "runtime/interfaceSupport.inline.hpp", in turns, includes "runtime/mutexLocker.hpp". Therefore, there is no need in having " #include "runtime/mutexLocker.hpp" statement in "src/hotspot/share/runtime/notificationThread.cpp" file since the header file "runtime/mutexLocker.hpp" is already included.
>
> Thanks,
> Daniil
>
> From: "serguei.spitsyn at oracle.com" <serguei.spitsyn at oracle.com>
> Date: Tuesday, July 9, 2019 at 1:37 AM
> To: Daniil Titov <daniil.x.titov at oracle.com>, OpenJDK Serviceability <serviceability-dev at openjdk.java.net>, "hotspot-runtime-dev at openjdk.java.net" <hotspot-runtime-dev at openjdk.java.net>, "jmx-dev at openjdk.java.net" <jmx-dev at openjdk.java.net>, David Holmes <david.holmes at oracle.com>
> Subject: Re: RFR: 8170299: Debugger does not stop inside the low memory notifications code
>
> Hi Daniil,
>
>
> On 7/8/19 16:24, Daniil Titov wrote:
> Hi Serguei,
>
> I will put it on hold as David asked but before doing so I just wanted to give a quick reply to the questions you asked.
>
> You did not answer my question about include:
> #include "runtime/mutexLocker.hpp"
>
> I'll provide a complete review after you sort out the David's concerns.
>
> Thanks,
> Serguei
>
> Thanks!
>
> Best regards,
> Daniil
>
>
>
> From: mailto:serguei.spitsyn at oracle.com mailto:serguei.spitsyn at oracle.com
> Date: Monday, July 8, 2019 at 3:09 PM
> To: Daniil Titov mailto:daniil.x.titov at oracle.com, OpenJDK Serviceability mailto:serviceability-dev at openjdk.java.net, mailto:hotspot-runtime-dev at openjdk.java.net mailto:hotspot-runtime-dev at openjdk.java.net, mailto:jmx-dev at openjdk.java.net mailto:jmx-dev at openjdk.java.net, David Holmes mailto:david.holmes at oracle.com
> Subject: Re: RFR: 8170299: Debugger does not stop inside the low memory notifications code
>
> Hi Daniil,
>
> Did you see a message from David Holmes?
> I do not see your reply.
>
> Specifically, David asked to hold on with this while he is on vacation for two weeks:
>
>> But introducing a new thread in the VM also has the same set of concerns!
>> This needs consideration by the runtime team before going ahead.
>> Introducing a new thread likes this needs to be examined in detail -
>> particularly the synchronization interactions with other threads.
>>
>> It also introduces another monitor designated safepoint-never at a time
>> when we are in the process of cleaning up monitors so that
>> JavaThreads will only use safepoint-check-always monitors.
>>
>> Unfortunately I'm about to head out for two weeks vacation, and
>> a number of other key runtime folk are also on vacation.
>> But I'd ask that you hold off on this until we can look at it in more detail.
>
> In fact, I was expecting this kind of concerns from David.
>
> Thanks,
> Serguei
>
>
> On 7/8/19 11:42, Daniil Titov wrote:
> Hi Serguei,
>
> Please review the new version of the fix that corrects the order of include statements in src/hotspot/share/runtime/notificationThread.cpp.
>
> The list of Include statements doesn't contain "#include "runtime/mutexLocker.hpp" since this include file is already included by runtime/interfaceSupport.inline.hpp that is in this list.
>
> I don't think we need the following function:
> static bool is_notification_thread(Thread* thread);
>
> For the ServiceThread the function is_service_thread(Thread* thread) is used only once in the code. It is used inside JVmtiDeferredEvent::post() to assert that the proper thread is used to post these events. Low memory, GC and diagnostic command notification never had such asserts so I'm not sure we need to introduce them regarding new NotificationThread.
>
> Webrev: https://cr.openjdk.java.net/~dtitov/8170299/webrev.02/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8170299
>
> Thanks!
> --Daniil
>
>
> On 7/3/19, 9:02 PM, mailto:serguei.spitsyn at oracle.com mailto:serguei.spitsyn at oracle.com wrote:
>
> 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
> >
> >
>
>
>
>
>
>
>
>
>
More information about the hotspot-runtime-dev
mailing list