Patches to java.util.zip by Christian Schlichtherle

[John Leuner]

More than 3 months ago I took on the work of helping Christian submit
his java.util.zip changes back into Classpath.

Unfortunately I haven't been able to give this task the attention it
deserves and I want to give someone else the opportunity to deal with
these patches.

Below are some of the emails exchanged between Christian, Mark Wielaard
and myself.

Attached are Christian's example ZipManager.java file mentioned below,
the java.util.zip diffs (which need code reformatting) in
jazzlibpatch001 
and 3 new files java/util/zip/CRC32If.java java/util/zip/DeflaterIf.java
java/util/zip/InflaterIf.java

John Leuner

On Mon, 2005-05-16 at 20:41, Christian Schlichtherle wrote:
> Hi everyone,
> 
> I recognise it's time for some explanations about the patches I've inducted.
> 
> I have started the work on this package as I need to support international
> ZIP files for my application http://truemirror.de/en/ and Sun's enforcement
> of UTF-8 is incompatible with other tools (like PKZIP). Here are some
> details:
> 
> + The additional constructors and methods for ZipFile, ZipInputStream and
> ZipOutputStream have been introduced to support international ZIP files,
> i.e. ZIP file entries using character sets other than US-ASCII (or UTF-8 as
> Sun's implementation enforces). Although the character set is not part of
> the ZIP file format standard definition of PKZIP, CP437 (the original PC
> character set) is used by all common tools for compatibility (other than
> Sun's JDK unfortunately).
> 
> + The CRC32If, InflaterIf and DeflaterIf interfaces have been introduced to
> enable a developer to select their favourite compression/decompression
> "engine" with the Zip* classes. This makes sense because although
> classpath's implementation is "100% pure Java" it is SIGNIFICANTLY slower
> than the native ZLIB implementation of Sun's JDK release for the Windows
> platform. If in doubt, you may do some tests using my application's
> "--jazzlib" command line option.
> 
> Using both features as a developer I can now use a ZIP file package which is
> 100% pure Java, supports international ZIP files and still benefits from the
> performance of Sun's JDK (at least on the Windows platform)!
> 
> Here is a brief excerpt from my application to illustrate what I mean.
> Please note that this code uses both packages net.sf.jazzlib and the genuine
> (JDK) java.util.zip:
> 
> <snippet>
> /* ... */
> package de.schlichtherle.nio;
> 
> /* ... */
> import net.sf.jazzlib.*;
> 
> class ZipManager /* ... */ {
>     /* ... */
> 
>     /** For use by address@hidden #determineCharSet(File)} only. */
>     private static String zipEncoding;
>     
>     /**
>      * @return The character set encoding to use for entry names in the
>      *         target ZIP or JAR file. Currently, this is <tt>"UTF-8"</tt>
>      *         for JAR files and <tt>"CP437"</tt> for ZIP files.
>      *         More intelligent heuristics may be added in future.
>      */
>     public final static String determineCharSet(final File target) {
>         if (File.isJarFileOnly(target))
>             return "UTF-8";
>         else {
>             if (zipEncoding == null) {
>                 try {
>                     zipEncoding = "CP437";
>                     new String("").getBytes(zipEncoding);
>                 }
>                 catch (UnsupportedEncodingException failure) {
>                     // Most probably international character sets have not
> been
>                     // installed on the JVM.
>                     zipEncoding = "UTF-8";
>                 }
>             }
>             return zipEncoding;
>         }
>     }
> 
>     /**
>      * A plain <tt>ZipFile</tt> object used for input from the device file.
>      * Also used to mount the file system.
>      */
>     protected LocalisedZipFile zipIn;
>     
>     /** The character set to use for ZIP or JAR file entry names. */
>     protected final String charSet;
>     
>     /** ... */
>     protected ZipManager(final File target)
>     throws IOException {
>         /* ... */
>         charSet = determineCharSet(target);
>     }
> 
>     /**
>      * This method ensures that the virtual ZIP or JAR file system is
> mounted
>      * and initialises <tt>fileSystem</tt> with it.
>      * ...
>      */
>     private void mountFileSystem(boolean autoCreate)
>     throws IOException {
>         /* ... */
>         zipIn = new LocalisedZipFile(deviceFile);
>         /* ... */
>     }
> 
>     /* ... */
> 
>     /**
>      * This class uses address@hidden java.util.zip.Inflater} to implement
>      * address@hidden InflaterIf}.
>      * This allows us to use native Inflater code from Sun's JDK for maximum
>      * performance.
>      */
>     protected static class JDKInflater
>             extends java.util.zip.Inflater
>             implements InflaterIf {
> 
>         public JDKInflater(boolean nowrap) {
>             super(nowrap);
>         }
>     }
> 
>     /**
>      * This class implements a localised <tt>ZipFile</tt>.
>      */
>     protected class LocalisedZipFile extends ZipFile {
>         
>         public LocalisedZipFile(java.io.File file)
>         throws  ZipException,
>                 IOException,
>                 UnsupportedEncodingException {
>             super(file, charSet);
>         }
> 
>         public final InputStream getInputStream(net.sf.jazzlib.ZipEntry
> entry)
>         throws IOException {
>             InflaterIf inf = useJazzlibCompression
>                            ? (InflaterIf) new net.sf.jazzlib.Inflater(true)
>                            : (InflaterIf) new JDKInflater(true);
>             return super.getInputStream(entry, inf);
>         }
>     }
> </snippet>
> 
> The inner class LocalisedZipFile does the trick: Depending on the command
> line switch "--jazzlib" it uses the (de)compression engine of either jazzlib
> or JDK (JDK is default for maximum performance). In either case, if the file
> to manage is a ZIP file (rather than a JAR file), the "standard" CP437 is
> used. This works for ALL countries.
> 
> As you can see, having the protected constructors/methods and additional
> interfaces makes perfect sense and thus I would very much like to see them
> in the next release. Moving them to another package may be a good option
> however.
> 
> About the "flush" method: I removed it because it isn't compatible with the
> introduced interfaces and Sun's JDK - unfortunately I don't remember the
> details here.
> 
> A comment on the "private final" changes: The "final" declaration is not
> only a statement to disallow overriding a method, but also a hint for the
> compiler to inline the method wherever it's called. The result is larger,
> but faster code as the method invocation overhead (stack and jump
> operations) is not required. You may validate my proposition with the
> following code:
> 
> <snippet>
> package test;
> 
> public class Main {
>     
>     private /*final*/ static void doit() {
>         System.out.println("Hello world!");
>     }
> 
>     public static void main(String[] args) {
>         doit();
>     }
> }
> </snippet>
> 
> Using Sun's JDK 1.5, the compiled file Main.class differs whether you
> uncomment the final declaration or not. Thus, careful use of "private final"
> results in better performance for jazzlib.
> 
> With best regards,
> Christian Schlichtherle
> ---
> Schlichtherle IT Services
> Wittelsbacher Str. 10a
> 10707 Berlin
>  
> Mobil: 0173 / 27 12 470
> mailto:address@hidden
> http://www.schlichtherle.de
> 
> -----Original Message-----
> From: Mark Wielaard [mailto:address@hidden 
> Sent: Sunday, May 15, 2005 10:25 PM
> To: John Leuner
> Cc: Christian Schlichtherle
> Subject: RE: Missing jazzlib files
> 
> Hi,
> 
> On Wed, 2005-05-04 at 09:24 +0700, John Leuner wrote:
> > Attached are the proposed changes to java/util/zip
> > 
> > I think that they need to be formatted by a tool (emacs or something
> > else) to match classpath style.
> > 
> > I haven't sent this to cp-patches because lists.gnu.org appears to be 
> > down.
> 
> And currently the mailinglist is fighting with a large number of German
> spam. Sigh. Anyway. Sorry for being so slow in responding. If you have been
> following planet.classpath.org you might have noticed the Apache Harmony and
> OpenOffice issues that took quite some of my time.
> 
> I didn't actually review the big part of the patch. And I don't think we
> should add new public/protected constructors or methods. We should probably
> move the new functionality to a new gnu.java.util.zip package with extended
> functionality if possible.
> 
> But I quickly went through some of the things that I could quickly format
> correctly and that I think can and should be submitted separately since they
> are good changes on their own anyway.
> 
> Reformatted Adler32.java to follow out style guide and make the diff minimal
> and updated copyright year. This can probably be submitted separately with a
> little ChangeLog message plus a short explanation of the why of the changed
> (any benchmarks?)
> 
> Dropped the change to java/util/zip/CRC32.java which I didn't understand
> (what is CRC32If?):
> -public class CRC32 implements Checksum
> +public class CRC32 implements CRC32If
> 
> Same for the changes to java/util/zip/Deflater.java (what is
> DeflaterIf?)
> 
> Dropped the change to java/util/zip/DeflaterEngine.java private methods are
> always final:
> -  private void updateHash() {
> +  private final void updateHash() {
> 
> Added copyright year to java/util/zip/DeflaterHuffman.java and changed order
> of modifiers to amtch coding styleguide. (Can also be submitted separately
> as obvious fix with ChangeLog entry.)
> 
> Removed the changes to java/util/zip/DeflaterOutputStream.java that involved
> DeflaterIf (which wasn't included and we cannot change the type and modifier
> of a protected field). Are you sure you want to comment out/remove the
> flush() method? (Can also be submitted separately with its own ChangeLog
> entry.)
> 
> Updated the copyright year in java/util/zip/Inflater.java and removed the
> implements InflaterIf. Updated documentation is obviously correct and can be
> submitted separately with a ChangeLog entry.
> 
> Remove the changes to java/util/zip/InflaterInputStream.java which all had
> to do with InflaterIf (not included).
> 
> The change to java/util/zip/OutputWindow.java is unnecessary since private
> methods are already final:
> -  private void slowRepeat(int rep_start, int len, int dist)
> +  private final void slowRepeat(int rep_start, int len, int dist)
> 
> java/util/zip/ZipException.java had as only change an update to the
> copyright years list. That is unnecessary.
> 
> I'll go through the rest later next week.
> 
> Please submit this to classpath-patches with a ChangeLog entry already.
> And if possible think about whether or not you can move the public interface
> changes to a new package/subclass.
> 
> Thanks,
> 
> Mark

ZipManager.java
Description: Text Data

jazzlibpatch001
Description: Text document

CRC32If.java
Description: Text Data

DeflaterIf.java
Description: Text Data

InflaterIf.java
Description: Text Data