RFR: 8193513: add support for printing a stack trace on class loading [v2]

[David Holmes]

On Fri, 30 Jun 2023 20:35:44 GMT, Doug Simon <dnsimon at openjdk.org> wrote:

>> Usually, the user would need to specify both. Otherwise just having the cause without the `class+load` would be rather useless
>> 
>> 
>> java -Xlog:class+load  -Xlog:class+load+cause 
>> 
>> 
>> Which can be combined like:
>> 
>> 
>> java -Xlog:class+load,class+load+cause 
>> 
>> 
>> or 
>> 
>> 
>> java -Xlog:class+load*=info
>> 
>> 
>> (To see usages of `-Xlog`, you can grep for `[-]Xlog:` in the *.java files under test/hotspot/jtreg)
>> 
>> 
>> In terms of implementing this PR, I would suggest handling the class+load and class+load+cause independently. Something like
>> 
>> 
>> 
>> void InstanceKlass::print_class_load_logging(ClassLoaderData* loader_data,
>>                                              const ModuleEntry* module_entry,
>>                                              const ClassFileStream* cfs) const {
>>   if (ClassListWriter::is_enabled()) {
>>     ClassListWriter::write(this, cfs);
>>   }
>> 
>>   log_class_load(...);  // handle -Xlog:class+load 
>>                         // (the rest of the old print_class_load_logging() function)
>>   log_class_load_cause(...);  // handle -Xlog:class+load
>> }
>> 
>> 
>> The output would be something like
>> 
>> 
>> [0.014s][info][class,load] java.lang.Thread$State source: shared objects file
>> [0.014s][info][class,load,cause] 	at jdk.internal.misc.VM.toThreadState(java.base/VM.java:336)
>> [0.014s][info][class,load,cause] 	at java.lang.Thread.threadState(java.base/Thread.java:2731)
>> [0.014s][info][class,load,cause] 	at java.lang.Thread.isTerminated(java.base/Thread.java:2738)
>> [0.014s][info][class,load,cause]	at java.lang.Thread.getThreadGroup(java.base/Thread.java:1957)
>> ...
>> 
>> 
>> This way, you don't need to repeat the class name, which is already printed by `-Xlog:class+load`
>
>> Also, this is always JavaThread::current() because all the callers have the current JavaThread. No need for the null check or the cast.
> 
> Does this mean this existing type check and cast is unnecessary: https://github.com/openjdk/jdk/blob/e8ff74c7e84ec2440a51fee1b4c45e87332807a0/src/hotspot/share/oops/instanceKlass.cpp#L3821-L3823

Yes. I originally raised the possibility of the current thread not being set yet, but then when I checked it would always be set. But you chose to to add in the check "just in case".

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

PR Review Comment: https://git.openjdk.org/jdk/pull/14553#discussion_r1249844732