RFR: 8291969: Convert LoaderConstraintsTable to ResourceHashtable [v5]

[Ioi Lam]

On Mon, 22 Aug 2022 14:31:29 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

>> This converts the LoaderConstraintTable to ResourceHashtable.  It's a bit different than the other conversions in that the LoaderConstraintEntry has to be restructured to hold all the loader constraints for that class name, since the search key is the class name.  Also, I replaced the raw arrays with GrowableArray, because it's less code.
>> Tested with tier1-3, and tier 4-7 previously with other changes.  I also verified code paths with some temporary asserts with existing tests, including jck tests.
>
> Coleen Phillimore has updated the pull request incrementally with one additional commit since the last revision:
> 
>   How about ConstraintSet ? It looks unconfusing in context.

Hi Coleen, I was confused by the diff in github, but after downloading the code and reading it in detail, I think the latest version looks reasonable. It's unfortunate that we had to change so much code, but this also gives up an opportunity to clean the code up and make it more maintainable.

The current logic looks good and equivalent to the old one. Some operations may be slower (e.g., deleting a Constraint requires an extra linear search) but hopefully there's little impact on real programs.

More renaming may be necessary (e.g. "entry" -> "set")  after we agree on the names of the types.

src/hotspot/share/classfile/loaderConstraints.cpp line 56:

> 54:   }
> 55: 
> 56:   ~LoaderConstraint() { delete _loaders; }

These two lines should be added to make sure that we never make a shallow copy of LoaderConstraint, or else the `_loaders` would be deleted multiple times.


  LoaderConstraint(const LoaderConstraint& src) = delete;
  LoaderConstraint& operator=(const LoaderConstraint&) = delete;

src/hotspot/share/classfile/loaderConstraints.cpp line 415:

> 413:   // Copy into the shorter of the constraints.
> 414:   LoaderConstraint* dest = p1->num_loaders() <= p2->num_loaders() ? p1 : p2;
> 415:   LoaderConstraint* src = dest == p1 ? p2 : p1;

Is the purpose to reduce the number of copies? If so, you should copy into the one with more loaders.

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

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