On Fri, 12 Sep 2025 15:48:16 GMT, Alan Bateman <[email protected]> wrote:
>> David Beaumont has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Tweak test.
>
> 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.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27203#discussion_r2350177094
