On Thu, 8 May 2025 03:22:32 GMT, Ioi Lam <ik...@openjdk.org> wrote: >> This fix adds the `--add-reads` support for CDS and AOTClassLinking. >> Before the fix, if the `--add-reads` is specified during CDS archive >> dumping, the user will see the following log if `-Xlog:cds=info` is enabled: >> `[0.000s][info][cds] optimized module handling: disabled due to incompatible >> property: jdk.module.addreads=com.norequires=org.astro` >> During runtime, the archived full module graph will be disabled: >> `[0.021s][info][cds ] full module graph: disabled` >> >> With the fix, the optimized module handling won't be disabled during dump >> time and the full module graph will be enabled during runtime provided the >> same --add-reads option is specified during dump time and runtime. >> >> Testing: tiers 1 - 4. > > I think we need a test to validate that the `--add-reads` flag actually > works. I wrote such a test when I implemented `--add-exports`: see > [runtime/cds/appcds/jigsaw/modulepath/AddExports.java](https://github.com/openjdk/jdk/blob/master/test/hotspot/jtreg/runtime/cds/appcds/jigsaw/modulepath/AddExports.java) > > I think we need a similar test for `--add-opens` as well. > > Since the main reason for supporting `--add-{expors,opens,reads}` is for user > of AOT class linking, I would suggest moving the above AddExports.java test > to > > - runtime/cds/appcds/aotClassLinking/modules/AddExports.java > > And then add AddReads.java and AddOpens.java there as well. > > The reason for putting the tests inside aotClassLinking is that they will be > excluded from the `hotspot_appcds_dynamic` and `hotspot_aot_classlinking` > test groups, so you don't need to add special case code to handle those two > test groups.
Thanks @iklam @AlanBateman for the review. ------------- PR Comment: https://git.openjdk.org/jdk/pull/25109#issuecomment-2885067118
