RFR(S) 8130448: thread dump improvements, comment, additions, new diagnostics inspired by 8077392
Daniel D. Daugherty
daniel.daugherty at oracle.com
Tue Jul 14 15:38:49 UTC 2015
Coleen,
Thanks for the review!
Replies embedded below.
On 7/14/15 8:18 AM, Coleen Phillimore wrote:
>
> Dan,
>
> This looks really good and a nice cleanup of wrong logging code (raw
> printfs) and an unused function (?).
Thanks!
For those that have been playing along at home, this bug fix is
coming from the "cleanup" bucket in the Contended Java Locking
project.
> I didn't read the whole thread but adding Knobs for printing seems
> consistent with the other knobs. I'd thought you'd added more command
> line arguments.
David H. rightly objected to me adding three more command line options
in Code Review Round 0 so we switched to using the already existing
SyncKnobs mechanism. As Misha has pointed out, we are under "cap and
trade" limits on command line options! :-)
> At some point, these logging knobs would be candidates for conversion
> to the new logging system (TBD).
And that's one of the reasons I switched from raw printfs to the
more normal tty->print_cr() stuff.
Thanks again for the review!
Dan
>
> Thanks,
> Coleen
>
> On 7/13/15 5:57 PM, Daniel D. Daugherty wrote:
>> Need a second reviewer for this bug fix.
>>
>> One minor tweak has been done relative to the webrev below:
>>
>> $ diff -c src/share/vm/runtime/synchronizer.cpp{.cr1,}
>> *** src/share/vm/runtime/synchronizer.cpp.cr1 Wed Jul 8 07:15:07 2015
>> --- src/share/vm/runtime/synchronizer.cpp Thu Jul 9 07:07:28 2015
>> ***************
>> *** 1651,1661 ****
>> Handle obj((oop) mid->object());
>> tty->print("INFO: unexpected locked object:");
>> javaVFrame::print_locked_object_class_name(tty, obj,
>> "locked");
>> }
>> - guarantee(ObjectMonitor::Knob_VerifyMatch == 0,
>> - err_msg("exiting JavaThread=" INTPTR_FORMAT
>> - " unexpectedly owns ObjectMonitor="
>> INTPTR_FORMAT,
>> - THREAD, mid));
>> (void)mid->complete_exit(CHECK);
>> }
>> }
>> --- 1651,1660 ----
>> Handle obj((oop) mid->object());
>> tty->print("INFO: unexpected locked object:");
>> javaVFrame::print_locked_object_class_name(tty, obj,
>> "locked");
>> + fatal(err_msg("exiting JavaThread=" INTPTR_FORMAT
>> + " unexpectedly owns ObjectMonitor="
>> INTPTR_FORMAT,
>> + THREAD, mid));
>> }
>> (void)mid->complete_exit(CHECK);
>> }
>> }
>>
>> Dan
>>
>>
>> On 7/8/15 3:33 PM, Daniel D. Daugherty wrote:
>>> Greetings,
>>>
>>> I've updated this patch based on David H's comments along with
>>> some additional cleanup.
>>>
>>> This work is being tracked by the following bug ID:
>>>
>>> JDK-8130448 8077392 inspired thread dump improvements, comment
>>> additions, new diagnostics
>>> https://bugs.openjdk.java.net/browse/JDK-8130448
>>>
>>> Here is the webrev URL:
>>>
>>> http://cr.openjdk.java.net/~dcubed/8130448-webrev/1-jdk9-hs-rt/
>>>
>>> The easiest way to review the delta is to download the two patch
>>> files and compare them in something like jfilemerge:
>>>
>>> http://cr.openjdk.java.net/~dcubed/8130448-webrev/0-jdk9-hs-rt/hotspot.patch
>>>
>>> http://cr.openjdk.java.net/~dcubed/8130448-webrev/1-jdk9-hs-rt/hotspot.patch
>>>
>>>
>>> Testing:
>>>
>>> - Aurora Adhoc RT/SVC nightly batches (in process)
>>> - JPRT test jobs
>>>
>>> Thanks, in advance, for any comments, questions or suggestions.
>>>
>>> Gory details about the changes are below...
>>>
>>> Dan
>>>
>>> Changes between CR0 and CR1 are:
>>>
>>> src/cpu/x86/vm/macroAssembler_x86.cpp
>>>
>>> - fix comment typos, clarify comment wording
>>>
>>> src/share/vm/oops/markOop.cpp
>>>
>>> - clarify output messages
>>> - get rid of confusing local variable
>>>
>>> src/share/vm/runtime/globals.hpp
>>>
>>> - drop VerboseStackTrace, GuaranteeOnMonitorMismatch
>>> and JavaThreadExitReleasesMonitors options
>>> - there are no longer changes to this file
>>>
>>> src/share/vm/runtime/objectMonitor.cpp
>>>
>>> - add ObjectMonitor::Knob_ExitRelease suboption to replace
>>> JavaThreadExitReleasesMonitors
>>> - add ObjectMonitor::Knob_VerifyMatch suboption to replace
>>> GuaranteeOnMonitorMismatch
>>> - cleanup messy logging:
>>> - switch from '::printf()' -> 'tty->print_cr()'
>>> - switch from '::fflush()' -> 'tty->flush()'
>>>
>>> src/share/vm/runtime/objectMonitor.hpp
>>>
>>> - add ObjectMonitor::Knob_ExitRelease suboption
>>> - add ObjectMonitor::Knob_VerifyMatch suboption
>>> - cleanup messy logging
>>>
>>> src/share/vm/runtime/synchronizer.cpp
>>>
>>> - cleanup messy logging
>>> - switch from GuaranteeOnMonitorMismatch to
>>> ObjectMonitor::Knob_VerifyMatch
>>> - add diagnostic information about the locked object
>>> when ReleaseJavaMonitorsClosure::do_monitor() finds
>>> a problem
>>>
>>> src/share/vm/runtime/thread.cpp
>>>
>>> - clarify comments
>>> - switch from JavaThreadExitReleasesMonitors to
>>> ObjectMonitor::Knob_ExitRelease
>>>
>>> src/share/vm/runtime/vframe.cpp
>>>
>>> - print_locked_object_class_name() is now public
>>> - clarify comments
>>> - clarify output messages
>>> - switch from VerboseStackTrace to the already existing
>>> ObjectMonitor::Knob_Verbose
>>>
>>> src/share/vm/runtime/vframe.hpp
>>>
>>> - print_locked_object_class_name() is now public
>>>
>>>
>>> On 7/3/15 6:14 PM, Daniel D. Daugherty wrote:
>>>> Greetings,
>>>>
>>>> The hunt for the following bug:
>>>>
>>>> JDK-8077392 Stream fork/join tasks occasionally fail to complete
>>>>
>>>> and David C's work on the following bug:
>>>>
>>>> JDK-8069412 Locks need better debug-printing support
>>>>
>>>> have inspired additional thread dump improvements, comment additions
>>>> to some Java monitor code and some new diagnostic options.
>>>>
>>>> This work is being tracked by the following bug ID:
>>>>
>>>> JDK-8130448 8077392 inspired thread dump improvements, comment
>>>> additions, new diagnostics
>>>> https://bugs.openjdk.java.net/browse/JDK-8130448
>>>>
>>>> Here is the webrev URL:
>>>>
>>>> http://cr.openjdk.java.net/~dcubed/8130448-webrev/0-jdk9-hs-rt/
>>>>
>>>> Testing:
>>>>
>>>> - RBT vm.quick batches (in process)
>>>> - JPRT test jobs
>>>>
>>>> Thanks, in advance, for any comments, questions or suggestions.
>>>>
>>>> Gory details about the changes are below...
>>>>
>>>> Dan
>>>>
>>>> 8130448 summary of changes:
>>>>
>>>> src/cpu/x86/vm/macroAssembler_x86.cpp
>>>> - comment additions for the assembly code
>>>>
>>>> src/share/vm/oops/markOop.cpp
>>>> - has_monitor() has to be checked before is_locked() since
>>>> the has_monitor() bits are a superset of the is_locked() bits
>>>> - code style fixes
>>>>
>>>> src/share/vm/runtime/globals.hpp
>>>> - add VerboseStackTrace diagnostic option
>>>> - add GuaranteeOnMonitorMismatch diagnostic option
>>>> - add JavaThreadExitReleasesMonitors product option;
>>>> this is the Java equivalent of JNIDetachReleasesMonitors
>>>>
>>>> src/share/vm/runtime/objectMonitor.cpp
>>>> - delete unused ObjectMonitor::try_enter()
>>>> - fix assert wording
>>>>
>>>> src/share/vm/runtime/objectMonitor.hpp
>>>> - delete unused ObjectMonitor::try_enter()
>>>>
>>>> src/share/vm/runtime/synchronizer.cpp
>>>> - add GuaranteeOnMonitorMismatch support with some
>>>> diagnostic output
>>>>
>>>> src/share/vm/runtime/thread.cpp
>>>> - add JavaThreadExitReleasesMonitors support
>>>>
>>>> src/share/vm/runtime/vframe.cpp
>>>> - clarify existing comments
>>>> - add comments to clarify what "waiting on" means
>>>> - add line to show when we are "waiting on", but
>>>> there are no locals available (previously we were
>>>> "waiting on" silently)
>>>> - add support for "waiting to relock" which can happen
>>>> for any of the monitors and not just the top monitor
>>>> - switch mark->print_on() verbose output to new
>>>> VerboseStackTrace switch; thread dumps are not enabled
>>>> with a specific switch so the '-XX:+Verbose' model of
>>>> being a modifier for another option does not fit
>>>>
>>>>
>>>
>>>
>>>
>>
>
More information about the hotspot-runtime-dev
mailing list