RFR: 8374308: ImageBufferCache does not work as intended

David Beaumont duke at openjdk.org
Mon Jan 5 17:36:39 UTC 2026


On Mon, 5 Jan 2026 17:19:50 GMT, David Beaumont <duke at openjdk.org> wrote:

> Remove ineffective/unused ImageBufferCache class, and simplify callers / remove dead code.
> 
> I removed the release methods in the internal classes, but the public ModuleReader API method is still there (the override can go away though since the default implementation also tests for non-null, so removing the override has no risk).
> 
> I suspect there are no implementations of ModuleReader that implement release semantics after this change, so perhaps we could relax the documentation around it? Thoughts welcome.

src/java.base/share/classes/jdk/internal/jimage/BasicImageReader.java line 390:

> 388:     }
> 389: 
> 390:     private static ByteBuffer allocateBuffer(long size) {

The only bit of code from ImageBufferCache that was being used in existing code.

src/java.base/share/classes/jdk/internal/jimage/BasicImageReader.java line 411:

> 409:     }
> 410: 
> 411:     /**

Seemed worth stressing that this is a new buffer allocation (since there's no "release" option to callers anymore, it's a usage/lifetime change).

src/java.base/share/classes/jdk/internal/jimage/ImageReader.java line 193:

> 191: 
> 192:     /**
> 193:      * Returns the content of a resource node in a newly allocated byte buffer.

While the "ByteBuffer" instance is newly allocated, it's almost always a cheap slice of the memory mapped file. I'm not sure what we want as the best wording here, since:
1. We want people to know they don't need to worry about "releasing" it, but also
2. we don't want people to think it's using a newly allocated/copied buffer.

Happy to take suggestion on better wording here and for "getResourceBuffer" above.

src/java.base/share/classes/jdk/internal/module/SystemModuleFinders.java line 470:

> 468:         }
> 469: 
> 470:         @Override

Can be removed since ModuleReader has a default method with the null check in it.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/29043#discussion_r2662257958
PR Review Comment: https://git.openjdk.org/jdk/pull/29043#discussion_r2662260718
PR Review Comment: https://git.openjdk.org/jdk/pull/29043#discussion_r2662270609
PR Review Comment: https://git.openjdk.org/jdk/pull/29043#discussion_r2662272689


More information about the core-libs-dev mailing list