RFR: 8291969: Convert LoaderConstraintsTable to ResourceHashtable [v4]

[Coleen Phillimore]

On Mon, 22 Aug 2022 07:33:26 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Rename LoaderConstraintEntry, which isn't a loader constraint, to ClassNameEntry.
>
> src/hotspot/share/classfile/loaderConstraints.cpp line 79:
> 
>> 77: 
>> 78: // For this class name, these are the LoaderConstraints for classes loaded with this name.
>> 79: class ClassNameEntry {                               // copied into hashtable as value
> 
> I find the naming for these map/table entries very awkward in general. A Map is a mapping from keytype K to a value type V. Here K ==  Symbol* (a class name), and V == a set of LoaderConstraints

I was trying to not repeat "LoaderConstraint" in the name of the hashtable entry of LoaderConstraints, because that's what made it look confusing (especially in the diff).
SetOfLoaderConstraints or LoaderConstraintSet.  Maybe ClassNameEntry is a terrible name.  I'm open for suggestions.

> src/hotspot/share/classfile/loaderConstraints.cpp line 170:
> 
>> 168:   // class that is eventually loaded must match between these two loaders.
>> 169:   bool created;
>> 170:   ClassNameEntry* entry = _loader_constraint_table.put_if_absent(name, &created);
> 
> Is this "empty" entry visible before we get to call initialize on it below? Why do we not construct the entry properly before adding it? Do we really expect concurrent additions in this case?

There is a lock around all accesses to this table.  This is a nicer pattern because we don't have to allocate, allocate again, copy, deallocate each entries with copy constructors.  We could do something fancy with the copy constructor and destructor (move semantics I think), this just seems easier to understand without tricky C++ code.

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

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