RFR: 8360037: Refactor ImageReader in preparation for Valhalla support
Roger Riggs
rriggs at openjdk.org
Tue Jul 1 18:57:44 UTC 2025
On Mon, 30 Jun 2025 23:27:49 GMT, David Beaumont <duke at openjdk.org> wrote:
> Refactoring `ImageReader` to make it easy to add preview mode functionality for Valhalla.
>
> This PR is a large change to `ImageReader` (effectively a rewrite) but reduces the surface area of the API significantly, reduces code complexity and increases performance/memory efficiency. The need for this sort of change comes from the need to support "preview mode" in Valhalla for classes loaded from core modules.
>
> ### Preview Mode
>
> In the proposed preview mode support for Valhalla, a resource with the name `/modules/<module>/<path>` may come from one of two locations in the underlying jimage file; `/<module>/<path>` or `/<module>/META-INF/preview/<path>`. Furthermore, directories (represented as directory nodes via the API) will have a potentially different number of child nodes depending on whether preview mode is in effect, and some directories may only exist at all in preview mode.
>
> Furthermore, the directories and symbolic link nodes in `/packages/...` will also be affected by the existence of new package directories. To retain consistency and avoid "surprises" later, all of this needs to be taken into account.
>
> Below is a summary of the main changes for mainline JDK to better support preview mode later:
>
> ### 1: Improved encapsulation for preview mode
>
> The current `ImageReader` code exposes the data from the jimage file in several ways; via the `Node` abstraction, but also with methods which return an `ImageLocation` instance directly. In preview mode it would not be acceptable to return the `ImageLocation`, since that would leak the existence of resources in the `META-INF/preview` namespace and lets users see resource nodes with names that don't match the underlying `ImageLocation` from which their contents come.
>
> As such, the PR removes all methods which can leak `ImageLocation` or other "underlying" semantics about the jimage file. Luckily most of these are either used minimally and easily migrated to using nodes, or they were not being used at all. This PR also removes any methods which were already unused across the OpenJDK codebase (if I discover any issues with over-trimming of the API during full CI testing, it will be easy to address).
>
> ### 2. Simplification of directory child node handling
>
> The current `ImageReader` code attempts to create parent directories "on demand" for any child nodes it creates. This results in parent directories having a non-empty but incomplete set of child nodes present. This is referred to as the directory being "incomple...
src/java.base/share/classes/jdk/internal/jimage/ImageReader.java line 555:
> 553:
> 554: /**
> 555: * Returns the JRY file system compatible name of this node (e.g.
Typo: "JRY"
src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java line 292:
> 290: @Override
> 291: public Optional<ModuleReference> find(String name) {
> 292: requireNonNull(name);
Personal preference is avoid static imports making the code readable by retaining the "Objects." qualifier otherwise the non-local method call appears out of thin air. YMMV.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/26054#discussion_r2178300589
PR Review Comment: https://git.openjdk.org/jdk/pull/26054#discussion_r2178311050
More information about the build-dev
mailing list