RFR(XS) 8249096: Clean up code for DumpLoadedClassList
Daniel D. Daugherty
daniel.daugherty at oracle.com
Fri Aug 21 19:18:08 UTC 2020
On 8/20/20 7:28 PM, Yumin Qi wrote:
> Hi, Ioi
>
> updated at new link:
> http://cr.openjdk.java.net/~minqi/2020/8249096/webrev-02/
> Summary of changes: 108 lines changed: 54 ins; 51 del; 3 mod;
14941 unchg
At >100 lines changed I wouldn't call this an XS review and it definitely
does not qualify as Trivial.
src/hotspot/share/oops/instanceKlass.hpp
No comments.
src/hotspot/share/classfile/classFileParser.cpp
No comments.
src/hotspot/share/classfile/systemDictionary.cpp
No comments.
src/hotspot/share/oops/instanceKlass.cpp
InstanceKlass::log_to_classlist() appears to be an equivalent
rewrite of the big logic block removed from classFileParser.cpp,
but that's really the call of someone that knows CDS better than
I do. Ioi has already said it's good so...
Thumbs up.
What testing was done on the webrev-02 version?
Dan
>
> Moved the call to 'log_to_classlist' to beginning of
> 'print_class_load_logging'. Removed the check for if class+load
> logging is enabled, since it will by pass the output for
> DumpLoadedClassList if not enabled.
>
> In 'print_class_load_logging', there exists a check for if the log
> is enabled.
>
> Thanks
>
> Yumin
>
>
> On 8/20/20 1:57 PM, Yumin Qi wrote:
>> Ioi,
>>
>> Thanks for the review, I will update with new webrev --- yes,
>> print_class_load_logging will work the same way.
>>
>>
>> Thanks
>>
>> YUmin
>>
>> On 8/20/20 11:37 AM, Ioi Lam wrote:
>>> Hi Yumin,
>>>
>>> This looks like a good clean up. I think it can be further
>>> simplified by moving the call to InstanceKlass::log_to_classlist to
>>> beginning of InstanceKlass::print_class_load_logging().
>>>
>>> Thanks
>>> - Ioi
>>>
>>> On 8/20/20 10:46 AM, Yumin Qi wrote:
>>>> Hi, Please review the very small change for cleaning up
>>>> DumpLoadedClassList code.
>>>>
>>>>
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8249096
>>>>
>>>> Webrev: http://cr.openjdk.java.net/~minqi/2020/8249096/webrev-01/
>>>>
>>>>
>>>> There are two places where when DumpLoadedClassList is set we log
>>>> loaded class to log file, one is in ClassFileParser::parse_stream,
>>>> and the other is in SystemDictionaryShared::load_shared_class_misc
>>>> which is after shared class loaded from CDS. The former is
>>>> positioned after parsing constants, and before parsing interface,
>>>> fields and methods etc. There is possibility an exception or error
>>>> happens before the InstanceKlass is created, so the fix move the
>>>> logging to after the InstanceKlass is successfully created, also
>>>> move the logging code to InstanceKlass which seems more reasonable.
>>>>
>>>>
>>>> Test: local jtreg on cds.
>>>>
>>>> Mach5 tier1-4
>>>>
>>>>
>>>> Thanks
>>>>
>>>> Yumin
>>>>
>>>
More information about the hotspot-runtime-dev
mailing list