RFR: 8262046: Clean up parallel class loading code and comments [v4]

[Coleen Phillimore]

On Thu, 1 Apr 2021 19:55:38 GMT, Lois Foltan <lfoltan at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Add comment about logging placeholders.
>
> src/hotspot/share/classfile/systemDictionary.cpp line 329:
> 
>> 327:   if (lt.is_enabled()) {
>> 328:     ResourceMark rm;
>> 329:     LogStream ls(lt);
> 
> Is it helpful to pass THREAD in for the ResourceMark?

Yes, I have a THREAD handy.

> src/hotspot/share/classfile/systemDictionary.cpp line 341:
> 
>> 339: //    parse_interfaces - from defineClass
>> 340: // super-class callers:
>> 341: //   ClassFileParser - from defineClass
> 
> picky review comment for line #340, comments below do not use a hyphenated "super-class", but instead one work "superclass".

Ok.  How about a space?  Because the sentence has super-class and super-interface, so I want to change them both to super class and super interface.  Or should it be superclass and superinterface ?

> src/hotspot/share/classfile/systemDictionary.cpp line 492:
> 
>> 490: // the superclass on the new thread internally, so we do parallel
>> 491: // super class loading here.  This avoids deadlock for ClassCircularity
>> 492: // detection for parallelCapable class loaders that lock a per-class lock.
> 
> "lock on a per-class lock"

Ok.

> src/hotspot/share/classfile/systemDictionary.cpp line 534:
> 
>> 532:       while (oldprobe != NULL &&
>> 533:              (oldprobe->instance_load_in_progress() || oldprobe->super_load_in_progress())) {
>> 534: 
> 
> I thought in preliminary discussions we talked about adding an assert to make sure instance_load_in_progress and super_load_in_progress couldn't both be true.  Am I remembering that correctly?

We were talking about when instance_load_in_progress is false, then we thought we should add an assert that super_load_in_progress is also false, but super_load could still be going on for parallel threads and we have to wait for them too.
ie. I tried this and found this out.  We have to wait for both conditions.

> src/hotspot/share/classfile/systemDictionary.cpp line 786:
> 
>> 784:           if (JvmtiExport::should_post_class_load()) {
>> 785:             JvmtiExport::post_class_load(THREAD->as_Java_thread(), loaded_class);
>> 786:           }
> 
> This seems to be all now handled in load_instance_class, correct?

Yes, I moved this down to that function.

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

PR: https://git.openjdk.java.net/jdk/pull/3200