RFR: 8367005: ImageReader refactor caused performance regressions for startup and footprint [v8]
Alan Bateman
alanb at openjdk.org
Tue Sep 16 06:19:13 UTC 2025
On Mon, 15 Sep 2025 21:38:29 GMT, David Beaumont <duke at openjdk.org> wrote:
>> src/java.base/share/classes/jdk/internal/jimage/ImageReader.java line 360:
>>
>>> 358: // underlying jimage, but can only reliably return resource nodes.
>>> 359: if (moduleName.indexOf('/') >= 0) {
>>> 360: return null;
>>
>> Can you explain this further? We can't have a "/' in the module name at the language or API level. We could at the class level but there is likely a lot of work required in jimage, jlink and elsewhere if we ever allowed "/" in a module name. So right now, it would be a IAE if a call if invoking this with a "/" in the module name.
>
> Do you mean explain it in this review thread, or in the code comment?
>
> I know module names shouldn't have a '/' in, but were someone to call it with such, then the constructed path could become valid (e.g. module name="java.base/java", resource path="lang/Object.class". Since I wasn't sure which of the callers can be trusted to pass only valid module names, I didn't want to make it an exception in case I caused something to blow up somewhere else.
>
> I'm happy to make it an IAE, I'm just trying to avoid code paths where this API gets given invalid input, but is fooled into giving back real data.
If this is called with "/" in a module name then it means we've got a bug somewhere. So I think the IAE would help find that bug quicker.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/27203#discussion_r2350977385
More information about the core-libs-dev
mailing list