On Thu, 9 Apr 2026 05:19:41 GMT, Eirik Bjørsnøs <[email protected]> wrote:
> This PR reorganizes `java.util.zip.ZipCoder` class hierarchies to handle > UTF-8 in the base class and move support for general charsets down to a > subclass. This better reflects reality where most ZIP files are encoded using > UTF-8. > > The motivation can be summarized as: > > * Supporting the common case in the base class and the exceptional case in a > subclass seems more natural and intuitive for maintainers > * We avoid loading both classes for most JVM instances > * A small performance improvement is observed, probably caused by unlocking > effecively-monomorphic JVM optimizations > > See core-libs-dev discussion for details: > https://mail.openjdk.org/pipermail/core-libs-dev/2026-January/158281.html > > I have tried to not move methods around too much, making the side-by-side > diff when ignoring whitespace easier to review. We can discuss further > cleanups and reorganizaton if so desired. > > It may be useful to review commits individually. The first commit fe4a627 > moves some methods out the way for the second commit 2926398. > > This PR also includes the similar change to the `ZipFileSystem` > implementation of ZipCoder to keep these aligned. Could be pulled out to a > separate PR if reviewers want to focus on a single area per PR. src/java.base/share/classes/java/util/zip/ZipCoder.java line 47: > 45: * for other charsets require external synchronization. > 46: */ > 47: class ZipCoder { I recommend adding some comments here about the hierarchy - the UTF8 is supported by the base class as the zip format requires some part of it to be handled by utf8, etc. In particular, note that ZipCoder instances in general are still stateful. src/java.base/share/classes/java/util/zip/ZipCoder.java line 248: > 246: if (asciiLen != len) { > 247: // Non-ASCII, fall back to decoding a String > 248: // We avoid using decoder() here since the UTF8ZipCoder is Dead comment src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipCoder.java line 77: > 75: static class CharsetZipCoder extends ZipCoder { > 76: > 77: static CharsetZipCoder UTF8_INSTANCE = new CharsetZipCoder(UTF_8); We don't need this guy. Just use the `String.getBytes(UTF_8)` and `new String(bytes, UTF_8)` instead. src/jdk.zipfs/share/classes/jdk/nio/zipfs/ZipCoder.java line 121: > 119: @Override > 120: boolean isUTF8() { > 121: return cs == UTF_8; Suggestion: return false; ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30640#discussion_r3057808547 PR Review Comment: https://git.openjdk.org/jdk/pull/30640#discussion_r3057873125 PR Review Comment: https://git.openjdk.org/jdk/pull/30640#discussion_r3057858841 PR Review Comment: https://git.openjdk.org/jdk/pull/30640#discussion_r3057866675
