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

[Ioi Lam]

On Thu, 1 Apr 2021 03:11:07 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> This change reduces the length of SystemDictionary::resolve_instance_class_or_null, the main class loading function to 4 pages on my screen, down from ~6.   Some functions are made static inside systemDictionary.hpp.
>> 
>> The only executable change is that handle_parallel_super_load used to have a wait while the class's superclass was being loaded after calling resolve_super_or_fail.  It was similar to the subsequent code that waited for the LOAD_INSTANCE placeholder, immediately following the return from handle_parallel_super_load.  I consolidated this code to wait for both conditions and kept the comment from handle_parallel_super_load.  This wait is now in the function handle_parallel_loading.
>> 
>> I also added a new load_instance_class that calls load_instance_class_impl.  The new load_instance_class does constraint checking and bookkeeping after it is called so that it can return with CHECK_NULL and not check pending exceptions. These functions aren't called outside of systemDictionary.cpp.
>> 
>> I added a test to show why handle_parallel_super_load is needed (see bug for details of the deadlock that this test would get if not for handle_parallel_super_load).
>> 
>> This updates comments to:
>> 1. rewrote ClassCircularityError detection above resolve_super_or_fail so that it made more sense to me (hope it does for others too).
>> 2. move comments to be near the code they describe
>> 3. removed some comments that referred to code somewhere else but not helpful at that location, or to describe code that was deleted or obsolete, or repetitive
>> 4. added a comment for resolve_instance_class_or_null
>> 
>> Ran tier1-3 testing, jck vm, jck lang and our internal parallel class loading tests.  Retesting with tiers 4-8 in progress (I tested an earlier version of this with no failures).
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Add comment about logging placeholders.

LGTM.

I like the refactoring that combines two similar code paths into the single `handle_parallel_loading` function. The subtle change in behavior probably doesn't matter, but if it did we should be able to catch it with 2+ months of testing before 17 RDP1.

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

Marked as reviewed by iklam (Reviewer).

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