On Mon, 21 Apr 2025 22:54:37 GMT, Calvin Cheung <cche...@openjdk.org> wrote:
>> src/java.base/share/classes/jdk/internal/module/ModuleBootstrap.java line >> 165: >> >>> 163: BootLoader.getUnnamedModule(); // trigger <clinit> of >>> BootLoader. >>> 164: >>> CDS.defineArchivedModules(ClassLoaders.platformClassLoader(), >>> ClassLoaders.appClassLoader()); >>> 165: boolean extraExportsOrOpens = >>> addExtraExportsAndOpens(bootLayer); >> >> (1) The returned value of `addExtraExportsAndOpens()` is not used. So I >> think this function can be changed to `void`, and all occurrences of the >> local variable `extraExportsOrOpens` can be removed. >> >> (2) I traced the code paths that depend on the effects of `--add-opens` and >> `--add-exports`. It Looks like some of the effects are recorded in the >> `java.lang.Module$ReflectionData::export` table: >> >> https://github.com/openjdk/jdk/blob/684d3b336e9cb31707d35e75f9b785e04e1fdbee/src/java.base/share/classes/java/lang/Module.java#L398C2-L412 >> >> >> /** >> * A module (1st key) exports or opens a package to another module >> * (2nd key). The map value is a map of package name to a boolean >> * that indicates if the package is opened. >> */ >> static final WeakPairMap<Module, Module, Map<String, Boolean>> >> exports = >> new WeakPairMap<>(); >> >> >> This table is *not* stored as part of the `ArchivedBootLayer`, so we must >> re-initialize this table in the production run. @AlanBateman could you >> confirm that this is correct. >> >> Eventually, we should enhance the `ArchivedBootLayer` to also include the >> tables in `Module$ReflectionData`. That will obviate the call to >> `addExtraExportsAndOpens()` and save a few bytecodes during start-up (but >> the overall impact would be small, so it's not critical in the current PR). >> Because these tables use WeakReference, we need to wait for >> [JDK-8354897](https://bugs.openjdk.org/browse/JDK-8354897). > > Fixed (1) above. > (2) I traced the code paths that depend on the effects of `--add-opens` and > `--add-exports`. It Looks like some of the effects are recorded in the > `java.lang.Module$ReflectionData::export` table: > > https://github.com/openjdk/jdk/blob/684d3b336e9cb31707d35e75f9b785e04e1fdbee/src/java.base/share/classes/java/lang/Module.java#L398C2-L412 > > ``` > /** > * A module (1st key) exports or opens a package to another module > * (2nd key). The map value is a map of package name to a boolean > * that indicates if the package is opened. > */ > static final WeakPairMap<Module, Module, Map<String, Boolean>> > exports = > new WeakPairMap<>(); > ``` > > This table is _not_ stored as part of the `ArchivedBootLayer`, so we must > re-initialize this table in the production run. @AlanBateman could you > confirm that this is correct. In the changes for "JEP draft: Prepare final means final", this is changed significantly so that reflectively exporting or opening a package during startup will add to openPackages/exportedPackages. Once the VM is fully initialized then reflective change via the addExports/addOpens API make use of ReflectionData. So no ReflectionData or weak refs setup for --add-exports/--add-opens. We could potential separate this out in advance so that ReflectionData doesn't need to be re-initialized. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/24695#discussion_r2053392827
