RFR: 8316967: Correct the scope of vmtimer in UnregisteredClasses::load_class [v2]

[Calvin Cheung]

On Tue, 10 Oct 2023 17:18:32 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> Calvin Cheung has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   correct the scope of the vmtimer
>
> In the non-CDS code, the `PerfClassTraceTime::CLASS_LOAD` event covers the call to `java.lang.ClassLoader::loadClass()`, so I think we should do that in `UnregisteredClasses::load_class()` as well.
> 
> https://github.com/openjdk/jdk/blob/33591a30d2e495b46877b76084aa2f52e5389246/src/hotspot/share/classfile/systemDictionary.cpp#L1294-L1319
> 
> I am trying to understand why I originally covered only the stream reading time but not the class parsing time. It was probably a bug :-)
> 
> https://github.com/openjdk/jdk/blob/4fd2a149977b05eb6e4b28d147ab9c043a7934ec/src/hotspot/share/classfile/classLoaderExt.cpp#L274-L295

> Changing the scope seems reasonable to me as well. Taking a close look of the current change after reading @iklam's comment above, I think it might be reasonable to change the `ClassLoader::perf_sys_class_lookup_time` name. Unregistered class loader is for user defined classloader support (continue paging back my memory for this specifically :)). Using `sys` could be confusing.

Thanks for the review and sorry for the delay in replying as I took a few days off.
How about changing `ClassLoader::perf_sys_class_lookup_time()` to `ClassLoader::perf_app_class_lookup_time()`?
Or simply using the existing `ClassLoader::perf_app_classload_time()`? (Which is being used in https://github.com/openjdk/jdk/blob/master/src/hotspot/share/classfile/systemDictionary.cpp#L1289-#L1299)

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

PR Comment: https://git.openjdk.org/jdk/pull/16046#issuecomment-1761800878