RFR: 8261268: LOAD_INSTANCE placeholders unneeded for parallelCapable class loaders

[David Holmes]

On Tue, 9 Feb 2021 01:20:04 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

> See CR for more details.  This optimizes some code for class loading.
> Tested with tier1-6.

Hi Coleen,

The actual logic changes seem sound (though some of the reorganisation was a little hard to follow).

I have some comments on comments, but overall this seems okay provided all testing passes.

Thanks,
David

src/hotspot/share/classfile/placeholders.cpp line 107:

> 105: // Doubly-linked list of Threads per action for class/classloader pair
> 106: // Class circularity support: links in thread before loading superclass
> 107: // bootstrap loader support:  links in a thread before load_instance_class

I don't think the loader is what the comment is referring to. The reference to bootstrapsearchpath relates to this comment in SD::resolve_instance_class_or_null:
    // add placeholder entry to record loading instance class
    // Five cases:
    // All cases need to prevent modifying bootclasssearchpath
    // in parallel with a classload of same classname

src/hotspot/share/classfile/systemDictionary.cpp line 594:

> 592:   // and has not yet finished.
> 593:   // In both cases the original caller will clean up the placeholder
> 594:   // entry on error.

Is this commentary really no longer applicable? While I don't fully understand it it does indicate the general conditions under which we will call handle_parallel_super_load.

src/hotspot/share/classfile/systemDictionary.cpp line 1865:

> 1863:     }
> 1864: 
> 1865:     DEBUG_ONLY(if (is_parallelCapable(class_loader)) verify_placeholder(name, loader_data));

I can't help but feel that somewhere in the loading process there should be a more general assertion that, not only is there a placeholder present, but that it is for this thread. Though such a check would also need to check the kind of placeholder found. Future RFE perhaps.

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

Marked as reviewed by dholmes (Reviewer).

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