RFR: 8298469: Obsolete legacy parallel class loading workaround for non-parallel-capable class loaders

[David Holmes]

On Mon, 13 Mar 2023 13:52:04 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> src/hotspot/share/classfile/placeholders.cpp line 137:
>> 
>>> 135:   assert(action != PlaceholderTable::LOAD_INSTANCE || !EnableWaitForParallelLoad || seen == nullptr,
>>> 136:          "Only one LOAD_INSTANCE allowed at a time");
>>> 137: 
>> 
>> Getting rid of the full assertion seems to go beyond obsoleting EnableWaitForParallelLoad.
>
> The assertion reduces to something somewhat useless.  Without EnableWaitForParallel load, you can have more then one thread with a LOAD_INSTANCE placeholder.  These threads will wait when the go call into ClassLoader.loadClass() now.
> There is nothing to assert here anymore.

Okay. I confess I don't fully grok this part though.

>> src/hotspot/share/classfile/systemDictionary.cpp line 564:
>> 
>>> 562: 
>>> 563: // For bootstrap and non-parallelCapable class loaders, check and wait for
>>> 564: // another thread to complete loading this class.
>> 
>> A replacement comment describing the method would be nice.
>
> Ok, how about
> // Check for other threads loading this class either to throw CCE or wait in the case of the boot loader.

Ok

>> src/hotspot/share/classfile/systemDictionary.cpp line 604:
>> 
>>> 602:         } else {
>>> 603:           return nullptr;
>>> 604:         }
>> 
>> Again unclear how this all disappears just because the flag is obsoleted.
>
> Only the bootclass loader waits on the SystemDictionary_lock for multiple threads now.  Not the non-parallel capable class loaders.

Got it. `must_wait_for_class_loading` implies boot-loader, so the original `return nullptr` is now handled outside the `if (must_wait_for_class_loading)`.

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

PR: https://git.openjdk.org/jdk/pull/12974