RFR: JDK-8177107: Reduce memory footprint of java.lang.reflect.Constructor/Method

[Joe Darcy]

On Thu, 3 Mar 2022 14:30:34 GMT, Claes Redestad <redestad at openjdk.org> wrote:

> This is also only a partial solution, since the initial array is created and injected by the VM on a "root" Constructor/Method, see for example the code for new method:
> 
> https://github.com/openjdk/jdk/blob/5c187e34a58769a129a0aae9e4937907c9060202/src/hotspot/share/runtime/reflection.cpp#L778
> 
> Depending on usage pattern most of the Constructor/Method objects created are such root objects (often created in bulk), and only those exposed to user code via a copy will benefit from this particular enhancement. Which might be a good optimization when getting the same Constructor/Method from the same class over and over.
> 
>     * Can we skip the `.clone()` when propagating these from a root object to one we'll expose? We have internal `getShared...` methods for that purpose.
> 
>     * If we update the JVM code to inject a reference to `Class.EMPTY_CLASS_ARRAY` and we can skip the `.clone()`ing as per above we should see a decent benefit overall.

I did also wonder to myself off-list if this style of optimization would be better handled in the JVM were the arrays of arguments are created. If so, I wouldn't feel comfortable implementing that in HotSpot sources myself.

> src/java.base/share/classes/java/lang/reflect/Executable.java line 59:
> 
>> 57: 
>> 58:     @SuppressWarnings({"rawtypes"})
>> 59:     static final TypeVariable[] NO_TYPE_VARS = new TypeVariable[0];
> 
> Since you asked me offline about some startup noise from this patch I took a look. This line might be another small contributor, since it means we're loading `jlr.TypeVariable` on JVM bootstrap. It would be nice if we could move this to `TypeVariable`, but we don't support private/package-private interface fields for some reason. Shouldn't we though, now that we have private interface fields, etc..?

IIRC, Java language-level private interface fields are on the to-do list, but are not a high priority.

> src/java.base/share/classes/java/lang/reflect/Executable.java line 61:
> 
>> 59:     static final TypeVariable[] NO_TYPE_VARS = new TypeVariable[0];
>> 60: 
>> 61:     static final Class<?>[] NO_TYPES = new Class<?>[0];
> 
> Less concerning since it doesn't add extra classloading, but I note that we have similar constants in a few places, including `j.l.Class.EMPTY_CLASS_ARRAY` and `j.l.r.ProxyGenerator.EMPTY_CLASS_ARRAY`. Perhaps would make sense to consolidate these into a single place/instance (and use consistent names)

A consolidated location like a non-instantiable java.lang.reflect.EmptyHolder class with static fields? (Exact name subject to bike shedding.)

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

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