RFR: 8292916: Streamline LoaderConstraint purging

[Coleen Phillimore]

On Fri, 26 Aug 2022 01:29:21 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> The code to purge loader constraints walks through the class loader list of an unloaded class, and then asserts that the there are no leftovers. It can just directly remove the list of loaders without that walk.
>> 
>> Also I left the variable name 'probe' to help with diffs for [JDK-8291969](https://bugs.openjdk.org/browse/JDK-8291969) a little, but it's a bad variable name.
>> 
>> Tested with tier1-3 which contains jck tests.
>
> src/hotspot/share/classfile/loaderConstraints.cpp line 106:
> 
>> 104:   }
>> 105: 
>> 106:   void remove_constraint(Symbol* name, LoaderConstraint* constraint) {
> 
> It seems wrong to pass in the name just so you can use it in a logging statement. Makes me think that `ConstraintSet` should also store the name for which it is a set of Constraints.

It would make the hashtable entries larger, so we don't want that.

> src/hotspot/share/classfile/loaderConstraints.cpp line 201:
> 
>> 199:         if (lt.is_enabled()) {
>> 200:           ResourceMark rm;
>> 201:           lt.print("purging class object from constraint for name %s,"
> 
> You are doing more than just purging the class object now.

Right.  This purges the entire constraint for the unloaded class object, without looping through the loader entries and doing it separately.  They should always be purged.

> src/hotspot/share/classfile/loaderConstraints.cpp line 213:
> 
>> 211:         // The class is alive or null but some of of the loaders may not be.
>> 212:         // Remove entries no longer alive from loader array
>> 213:         for (int n = constraint->num_loaders() - 1; n >= 0; n--) {
> 
> I'm curious why we iterate backwards here?

We iterate backwards so that I can remove the entries at the end first and can still use indexing to loop through the growable array.  We do that with growable arrays in a lot of places.

> src/hotspot/share/classfile/loaderConstraints.cpp line 234:
> 
>> 232:         }
>> 233:         // Check whether the set should be purged
>> 234:         // Only one loader doesn't enforce a constraint.
> 
> So just to be clear, if we then have a second loader for a class of this name, then that would cause a constraint to be recreated, which would include both loaders?

Yes. This is what it does.  I had to figure this out with a bunch of asserts...

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

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