On Wed, 22 Feb 2023 13:38:57 GMT, Volker Simonis <simo...@openjdk.org> wrote:
>> I also have to disagree with the statement. The unit of unloading is the >> ClassLoader. > >> I also have to disagree with the statement. The unit of unloading is the >> ClassLoader. > > Hidden classes are not normal classes. They are not defined, created or > loaded by a class loader. The only reason why hidden classes maintain a link > to a *defining class loader* is because they need it to resolve types used by > the hidden class's own fields and methods. > > Some references from [JEP 371: Hidden Classes](https://openjdk.org/jeps/371): > >> Dynamically generated classes may only be needed for a limited time, so >> retaining them for the lifetime of the statically generated class might >> unnecessarily increase memory footprint. Existing workarounds for this >> situation, such as per-class class loaders, are cumbersome and inefficient. > >> A hidden class can be unloaded when it is no longer reachable, or it can >> share the lifetime of a class loader so that it is unloaded only when the >> class loader is garbage collected > >> A hidden class is not created by a class loader and has only a loose >> connection to the class loader deemed to be its defining loader. We can turn >> these facts to our advantage by allowing a hidden class to be unloaded even >> if its notional defining loader cannot be reclaimed by the garbage collector. > >> By default, Lookup::defineHiddenClass will create a hidden class that can be >> unloaded regardless of whether its notional defining loader is still alive. >> That is, when all instances of the hidden class are reclaimed and the hidden >> class is no longer reachable, it may be unloaded even though its notional >> defining loader is still reachable. This behavior is useful when a language >> runtime creates a hidden class to serve multiple classes defined by >> arbitrary class loaders: The runtime will see an improvement in footprint >> and performance relative to both `ClassLoader::defineClass()` and >> Unsafe::defineAnonymousClass()` > > The only reason why hidden classes created by `LambdaMetaFactory` are > strongly linked to a class loader (at least I haven't heard any other > argument until now in this thread) is to *save native memory* and not because > it is *logically required*! It's fine for me if you don't want to fix that. I > can just not understand why you are all still insisting that creating a new > ClassLoaderData object per hidden class is such a great decision and fixing > that would not be beneficial. > > Hidden classes were designed to be light-weight and easily unloadable (see > JEP references above). In the case of `LambdaMetaFactory` we unnecessarily > link them strongly to a class loader just because the current implementation > is too expensive otherwise. On a side note, the JDK already creates > non-strongly linked hidden classes as well, e.g. for > `java.lang.invoke.MethodHandles$Lookup.unreflect()`. > @simonis I want to ask a basic question -- what is the reason for your code > to call `LambdaMetafactory.metafactory()` directly? It looks like you want to > implement so sort of dynamic dispatch. Can equivalent functionality be > implemented by the app itself? > > If there's a real need for such a style of programming, and it requires some > sort of built-in support in by the JDK, maybe we should have a proper API > instead of piggy-backing on `LambdaMetafactory.metafactory()`. > > I think if you give us more background, we can make this a more productive > discussion than focusing on "did we make the right decision N versions ago" > without defining what "right" means :-) > > I would suggest re-booting this decision in the mailing lists rather than > continuing in this PR. The initial reason for this issue comes from here https://github.com/aws/aws-sdk-java-v2/issues/3701. That issue could be solved by caching. However, as I've outlined in my other answers above, I still think that one ClassLoaderData per hidden class is overly expensive and not really required. I'm fine if you don't want to fix that I just don't understand why you think it is a great solution? ------------- PR: https://git.openjdk.org/jdk/pull/12493