On Wed, 22 Jan 2025 03:28:00 GMT, Ioi Lam <ik...@openjdk.org> wrote:
> I reimplemented
> `SystemDictionaryShared::lookup_super_for_unregistered_class()` with Java
> code. This removes hacks in
> `SystemDictionary::resolve_with_circularity_detection()` to facilitate future
> clean-ups there that are planned by @coleenp. Please see the [JBS
> issue](https://bugs.openjdk.org/browse/JDK-8348240) for a discussion of why
> the hack was there.
>
> The new Java code is in the `jdk/internal/misc/CDS$UnregisteredClassLoader`
> class. I also simplified `UnregisteredClasses::create_classloader()` by
> moving some unwieldy C++ `JavaCalls` code into Java.
>
> ### How this works:
>
> For example, let's assume you have the following classfiles in foo.jar:
>
>
> interface MyInterface1 {}
> interface MyInterface2 {}
> class MyClass extends Object implements MyInterface1, MyInterface2 {}
>
>
> The CDS classlist file can look like this:
>
>
> java/lang/Object id: 1
> MyInterface1: id: 2 super: 1 source: foo.jar
> MyInterface2: id: 3 super: 1 source: foo.jar
> MyClass: id: 4 super: 1 interfaces: 2 3 source: foo.jar
>
>
> When CDS handles the `MyClass: id: 4` line, it calls
> `CDS$UnregisteredClassLoader::load()` with the following parameters:
>
> - `name` = "MyClass"
> - `superClass` = java.lang.Object
> - `interfaces` = {MyInterface1, MyInterface2}
>
> `CDS$UnregisteredClassLoader::load()` will start parsing MyClass.class from
> foo.jar. The ClassFileParser will see symbolic references to the supertypes
> of `MyClass`. These supertypes will be resolved by
> `CDS$UnregisteredClassLoader::findClass()`, using the classes provided by
> `superClass` and `interfaces`.
Looks like a good cleanup. I have couple of comments.
src/hotspot/share/cds/unregisteredClasses.cpp line 72:
> 70:
> 71: JavaValue result(T_OBJECT);
> 72: JavaCallArguments args(2);
Should the 2 be increased to 3?
src/hotspot/share/classfile/vmSymbols.hpp line 742:
> 740: template(toFileURL_name, "toFileURL")
> \
> 741: template(toFileURL_signature,
> "(Ljava/lang/String;)Ljava/net/URL;") \
> 742: template(url_array_classloader_void_signature,
> "([Ljava/net/URL;Ljava/lang/ClassLoader;)V") \
I think you can also remove the following since `URLClassLoader` is not used
directly anymore:
`do_klass(URLClassLoader_klass, java_net_URLClassLoader
) `
-------------
PR Review: https://git.openjdk.org/jdk/pull/23226#pullrequestreview-2568629771
PR Review Comment: https://git.openjdk.org/jdk/pull/23226#discussion_r1926215557
PR Review Comment: https://git.openjdk.org/jdk/pull/23226#discussion_r1926205535
