On Sat, 24 Aug 2024 10:49:56 GMT, Eirik Bjørsnøs <[email protected]> wrote:
> Please review this refactoring PR which moves the `ZipEntry.flags` field to > `ZipOutputStream.XEntry`. > > Moving this field will save four bytes from the `ZipEntry` object size and > also saves an unneccessary read in `ZipFile.getZipEntry`. > > Testing: > > This PR is a refactoring of existing code and does not update any tests. I > added the label `noreg-cleanup` to the JBS issue. > > The following runs clean: > > > make test TEST="test/jdk/java/util/zip" > make test TEST="test/jdk/java/util/jar" > > > Performance: > > The JMH benchmark `java.util.zip.ZipFileGetEntry.getEntryHit` show a small > but consistent improvement (2-3%). I need to spend some more time thinking about this, but I can ask what made you decide to look at this? Changing the field name from _flag_ -> _flags_ then becomes a bit confusing as for example its usage in initCEN and the name of CENFLG. The field is also used via ZipInputStream (not via ZipEntry but _flag_). So some thought needs to be given to the naming Similar comment for ZipFileSystem as if we move forward, then we need to be consistent in naming otherwise it becomes (or can be) confusing for future maintainers. Also the Zip Spec refers to the field as "general purpose bit flag" and the zlib repository includes a minzip, which also references this field as _flag_ which might be another reason to keep the field name as is regardless of it moving to XEntry ------------- PR Comment: https://git.openjdk.org/jdk/pull/20702#issuecomment-2308908970
