On Mon, 29 Sep 2025 14:43:23 GMT, Roger Riggs <[email protected]> wrote:
>> src/java.base/share/native/libjimage/imageFile.hpp line 259:
>>
>>> 257: // it exists in the same module.
>>> 258: FLAGS_IS_PREVIEW_ONLY = 0x4
>>> 259: };
>>
>> A refactoring of the flags can avoid conflicting states.
>> FLAGS_IS_PREVIEW_VERSION = 2; // set for preview version
>> FLAGS_NO_NORMAL_VERSION = 1; // preserving the zero value for normal and
>> no preview.
>>
>> Create static methods in ImageLocation to perform the desired tests on flags.
>> The utility methods make the code easier to read.
>> Also define macros/function in the native code in ImageFile and jimage.
>>
>> isPreviewVersion(flags) { return (flags & FLAGS_IS_PREVIEW_VERSION) != 0)}
>> isPreviewOnly(flags) { return (flags & FLAGS_IS_REVIEW_VERSION |
>> FLAGS_NO_NORMAL_VERSION) == FLAGS_NO_NORMAL_VERSION}
>
> Reprise:
>
> Your name FLAGS_IS_PREVIEW_ONLY probably better than FLAGS_NO_NORMAL_VERSION
> and has the same semantics.
>
> FLAGS_HAS_PREVIEW_VERSION means that there is a preview version and it will
> be found in the <META-INF/Preview> hierarchy. if the flag is in the preview
> node then this is it. If found in the normal hierarchy, the preview class is
> found in the <META-INF/Preview> hierarchy.
The FLAGS_HAS_PREVIEW_VERSION flag is only set on non-preview locations. It's
what indicates that you may need to look for a preview version in preview mode
(rare but possible).
For C++ I think this occasional 2nd hash calculation and lookup shouldn't be an
issue because it's for class loading, so each successful lookup triggers a lot
of additional work (and as I understand it, failed lookups should not be common
in this code -- as opposed to the JRT file system code).
There are several possible approaches to address this is it does turn out to be
a performance issue though.
-------------
PR Review Comment:
https://git.openjdk.org/valhalla/pull/1618#discussion_r2413582640
