RFR: 8367005: ImageReader refactor caused performance regressions for startup and footprint [v8]
Jaikiran Pai
jpai at openjdk.org
Fri Sep 12 15:04:20 UTC 2025
On Fri, 12 Sep 2025 12:08:35 GMT, David Beaumont <duke at openjdk.org> wrote:
>> Summary: Add two new methods to ImageReader to make SystemModuleReader more performant.
>>
>> Analysis of benchmarks shows that when the vast majority of resources (~11,000 in the Perfstartup-SwingSet-G1 benchmark) tested for in SystemModuleReader do NOT exist, performance is degraded compared to this code prior to the refactoring in JDK-8360037.
>>
>> The current refactoring of ImageReader has everything going through a single "findNode()" method for simplest possible encapsulation, but while this is functionally correct, it's not tuned for testing for the non-existence of resources.
>>
>> In particular:
>> 1. SystemModuleReader only requests resources (i.e. things in the jimage file with paths *not* starting /modules/ or /packages/). This means findNode() does two look-ups for the resource, the first of which will always fail.
>> 2. The containsResource() logic doesn't need to create and cache nodes in ImageReader, it can just check for the presence of an ImageLocation corresponding to a resource.
>>
>> Thus two new methods are added to resolve these cases:
>> * findResourceNode(module, path)
>> * containsResource(module, path)
>>
>> Their API takes module and path separately so as to not be confusable with findNode(nodename).
>>
>> However care must be taken to prevent these methods being fooled into returning non-resource entries (this was possible before the refactoring by using module names like "modules" or "packages") so new tests have been added.
>
> David Beaumont has updated the pull request incrementally with one additional commit since the last revision:
>
> Tweak test.
test/jdk/jdk/internal/jimage/ImageReaderTest.java line 163:
> 161: // Or module names/paths to be empty.
> 162: "'':modfoo/com/foo/Alpha.class",
> 163: "modfoo:''"})
Does the use of `'` literals in that string translate to an empty value for module/resource path? I haven't checked myself.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27203#discussion_r2344520343
More information about the core-libs-dev
mailing list