On Fri, 12 Sep 2025 12:08:35 GMT, David Beaumont <[email protected]> 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
