RFR: 8368333: [lworld] Add preview mode to ImageReader and JRT file-system [v3]
David Beaumont
duke at openjdk.org
Wed Oct 8 19:58:15 UTC 2025
On Wed, 24 Sep 2025 18:54:02 GMT, Roger Riggs <rriggs at openjdk.org> wrote:
>> David Beaumont has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Manually deleting ImageReaderFactory (it returned somehow)
>
> src/java.base/share/classes/jdk/internal/jimage/ImageReader.java line 333:
>
>> 331: // it references have that package marked as preview only.
>> 332: // Skipping these entries avoids empty package subdirectories.
>> 333: if (previewMode || (flags & FLAGS_IS_PREVIEW_ONLY) == 0) {
>
> Add utility methods for FLAGS_IS_PREVIEW_ONLY == 0
Done (in next push).
> src/java.base/share/classes/jdk/internal/jimage/ModuleReference.java line 122:
>
>> 120: public boolean hasContent() {
>> 121: return ((flags & FLAGS_HAS_CONTENT) != 0);
>> 122: }
>
> isEmpty could avoid introducing "content" as a loosely defined term.
> It would be more similar to directories being empty or lists being empty.
I've bounced between this several times. The thing is that a non-empty package directory will not have content if it only contains other directories, and seeing logic talking about a package being "empty" when it has child directories is weird/confusing. I've actually thought about this naming quite hard and I think it's more misleading to use the term "empty". However, since it's obviously still not clear enough maybe a different name altogether?
How about "hasResources()" or "hasResourceContent()" ?
> src/java.base/share/classes/jdk/internal/jimage/PreviewMode.java line 65:
>
>> 63: FOR_RUNTIME() {
>> 64: @Override
>> 65: boolean resolve() {
>
> Each of these overrides creates an extra subclass.
> The alternative is for the resolve method use a switch on the ordinal to return or compute the boolean.
Excellent point, I'll fix this in the next push.
-------------
PR Review Comment: https://git.openjdk.org/valhalla/pull/1619#discussion_r2414886599
PR Review Comment: https://git.openjdk.org/valhalla/pull/1619#discussion_r2414884864
PR Review Comment: https://git.openjdk.org/valhalla/pull/1619#discussion_r2414885908
More information about the valhalla-dev
mailing list