RFR: 8292699: Improve printing of classes in native debugger [v11]

Coleen Phillimore coleenp at openjdk.org
Mon Oct 24 18:12:51 UTC 2022


On Sun, 23 Oct 2022 05:31:12 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> src/hotspot/share/interpreter/bytecodeTracer.cpp line 171:
>> 
>>> 169: void BytecodeTracer::trace_interpreter(const methodHandle& method, address bcp, uintptr_t tos, uintptr_t tos2, outputStream* st) {
>>> 170:   if (TraceBytecodes && BytecodeCounter::counter_value() >= TraceBytecodesAt) {
>>> 171:     ttyLocker ttyl;  // 5065316: keep the following output coherent
>> 
>> We should file a a RFE to make this a leaf mutex rather than ttyLocker (which does get broken to allow safepoints!) with no_safepoint_check assuming that the printing doesn't safepoint.  Good to remove the no-longer accurate comment.
>
> The comment also says:
> 
> 
>     // Using the ttyLocker prevents the system from coming to
>     // a safepoint within this code, which is sensitive to Method*
>     // movement.
> 
> 
> So are we trying to "prevent Method movement"? Does this even make sense after we switched to Permgen?

Nope doesn't make sense.

>> src/hotspot/share/utilities/debug.cpp line 664:
>> 
>>> 662: }
>>> 663: 
>>> 664: extern "C" JNIEXPORT void printclass(intptr_t k, int flags) {
>> 
>> It's weird that the function to print the method is findmethod, and the one to print the class is printclass. I see that findmethod is consistent with the 'find' functions above.  Maybe this should be findclass() too.
>
> I had two groups of printing functions with the following intention:
> 
> - findclass/findmethod will search for class/methods by their names. Call these when you don't have an InstanceKlass* or Method*
> - printclass/printmethod should be used when you already have an InstanceKlass* or Method*
> 
> However, I realized that the second group of methods should really be something like InstanceKlass::print_xxx() and Method::print_xxx(), so you can do something like this in gdb 
> 
> 
> call ik->print_all_methods_with_bytecodes();
> 
> 
> I have removed the second group of functions from this PR (commit [6675901](https://github.com/openjdk/jdk/pull/9957/commits/6675901125018853bad316a62702a8f7c62f331b)). I may add them in a follow-up PR if necessary.

Yes, I agree.  If you already have the InstanceKlass, it should be a function using that as a 'this' pointer.

-------------

PR: https://git.openjdk.org/jdk/pull/9957


More information about the hotspot-dev mailing list