On Fri, 4 Apr 2025 06:15:27 GMT, Ioi Lam <ik...@openjdk.org> wrote: >> Timofei Pushkin has updated the pull request incrementally with two >> additional commits since the last revision: >> >> - Make getResource concurrent-friendly >> - Don't use URLClassLoader > > src/java.base/share/classes/jdk/internal/misc/CDS.java line 420: > >> 418: // class loader. Thus it is safe to delegate their >> loading to system class loader >> 419: // (our parent) - this is what the default >> implementation of loadClass() will do. >> 420: return (Class<?>) DEFINE_CLASS.invoke(this, name, res); > > Instead of subclassing from `URLClassLoader` and go through the trouble of > calling its `defineClass()` method, maybe we should just subclass from > `ClassLoader` and maintain a hashtable of java.util.jar.JarFiles. > > > HashMap<String, JarFile> map = ....; > JarFile jar = map.get(source) ... or open a new JarFile if not found > byte[] buffer = read jar file for the given class name; > call ClassLoader.defineClass(buffer, ...)
All the opening and reading is handled by `URLClassPath` (it's not just JARs, can also be directories). I used only `defineClass` from `URLClassLoader` to minimize the behavior difference with the old code — besides defining the class it defines its package and protection domain. But it looks like static CDS strips these anyway, so I've removed `URLClassLoader` entirely. `URLClassPath` usage can also be removed — then the small addition to it from this PR won't be needed but its functionality will be somewhat duplicated in `CDS.UnregisteredLoader`. I've implemented this [in a separate branch](https://github.com/openjdk/jdk/compare/master...TimPushkin:jdk:one-loader-v2?expand=1). ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24223#discussion_r2029253444
