RFR: 8338931: ZipEntry.flags could be made internal to ZipOutputStream

Lance Andersen lancea at openjdk.org
Sun Aug 25 16:12:01 UTC 2024


On Sat, 24 Aug 2024 10:49:56 GMT, Eirik Bjørsnøs <eirbjo at openjdk.org> 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


More information about the core-libs-dev mailing list