On Mon, 15 Sep 2025 21:33:15 GMT, David Beaumont <[email protected]> wrote:
>> src/java.base/share/classes/jdk/internal/jimage/ImageReader.java line 381:
>>
>>> 379: /** Returns whether a resource exists in the given module. */
>>> 380: boolean containsResource(String moduleName, String
>>> resourcePath) {
>>> 381: // This method is tuned for cases where resources are
>>> likely to NOT
>>
>> I think this sentence can move to the method description. Also I'm not sure
>> about saying "NOT exist" as it's really just an existence check. The system
>> ModuleFinder find method will mostly be asked to locate resources that are
>> in module packages so they will exist.
>
> That's not what I was seeing in the logging for the benchmark. >10,000
> lookups, with about 20 that existed.
> Now, obviously if the benchmark is deliberately pushing non-existent
> resources, then it's not typical, but these were things like "button.gif"
> being searched for in all modules from the Swing code by the look of it.
> I don't mind removing this if this sort of behaviour isn't normal, or I can
> reword it to warn about the _possibility_ of lots of missing resources. What
> this is here for is so that a future maintainer doesn't optimise/modify it
> with the assumption that finding resources is the expected outcome.
The comment was about putting the useful comment into the method description,
so not suggesting changing the method body.
As regards the benchmark then it's just a Class.getResource usage from code on
the class path. The class loader delegation means the built-in class loaders
are being called to locate a resource that doesn't map to a module package.
This is the slow case and forces all modules to be opened to find the resource.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27203#discussion_r2351040103
