Hi, On Sun, 2005-07-24 at 12:29 +0700, John Leuner wrote: > 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.
I went through what I reviewed earlier and committed all uncontroversial
stuff.
> > 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?)
I haven't done any benchmarks myself. We probably should also have a
little mauve tests for Adler32 to make sure we don't introduce any bugs.
Cleaned up patch attached.
> > 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.)
Commited as:
2005-08-30 Christian Schlichtherle <[EMAIL PROTECTED]>
* java/util/zip/DeflaterHuffman.java (bit4Reverse): Mark final.
> > 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.)
Dropped the removal of flush() but kept the increased buffer size.
Committed as:
2005-08-30 Christian Schlichtherle <[EMAIL PROTECTED]>
* java/util/zip/DeflaterOutputStream.java
(DeflaterOutputStream(OutputStream)): Increase buffer size to 4096.
(DeflaterOutputStream(OutputStream,Deflater)): Likewise.
> > 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.
A similar fix was already applied earlier:
2005-07-13 David Gilbert <[EMAIL PROTECTED]>
* java/util/zip/Inflater.java: minor API doc fixes.
> > 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 need to go through the rest of these patches.
It would help if they could be rediffed against current CVS head and
extra functionality would be moved to a new package.
Thanks,
Mark
Index: java/util/zip/Adler32.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/util/zip/Adler32.java,v
retrieving revision 1.5
diff -u -r1.5 Adler32.java
--- java/util/zip/Adler32.java 22 Jan 2002 22:27:02 -0000 1.5
+++ java/util/zip/Adler32.java 15 May 2005 19:46:45 -0000
@@ -1,5 +1,5 @@
/* Adler32.java - Computes Adler32 data checksum of a data stream
- Copyright (C) 1999, 2000, 2001 Free Software Foundation, Inc.
+ Copyright (C) 1999, 2000, 2001, 2005 Free Software Foundation, Inc.
This file is part of GNU Classpath.
@@ -153,6 +153,19 @@
}
/**
+ * In update() we can defer the modulo operation:
+ * s1 maximally grows from 65521 (BASE) to 65521 + 255 * 3800
+ * s2 maximally grows by 3800 * median(s1) = 2090079800 < 2^31
+ * Calculated this value like this:
+ *
+ * int i = 0, a = BASE, b = a;
+ * for(; b > 0; i++)
+ * b += (a += 255);
+ * System.out.println("max defer = " + (i - 1));
+ */
+ private static final int DEFER = 3850;
+
+ /**
* Updates the checksum with the bytes taken from the array.
*
* @param buf an array of bytes
@@ -161,20 +174,13 @@
*/
public void update (byte[] buf, int off, int len)
{
- //(By Per Bothner)
int s1 = checksum & 0xffff;
int s2 = checksum >>> 16;
-
- while (len > 0)
+
+ while (off < len)
{
- // We can defer the modulo operation:
- // s1 maximally grows from 65521 to 65521 + 255 * 3800
- // s2 maximally grows by 3800 * median(s1) = 2090079800 < 2^31
- int n = 3800;
- if (n > len)
- n = len;
- len -= n;
- while (--n >= 0)
+ int max = Math.min(off + DEFER, len);
+ while (off < max)
{
s1 = s1 + (buf[off++] & 0xFF);
s2 = s2 + s1;
@@ -182,16 +188,6 @@
s1 %= BASE;
s2 %= BASE;
}
-
- /*Old implementation, borrowed from somewhere:
- int n;
-
- while (len-- > 0) {
-
- s1 = (s1 + (bs[offset++] & 0xff)) % BASE;
- s2 = (s2 + s1) % BASE;
- }*/
-
checksum = (s2 << 16) | s1;
}
Index: java/util/zip/DeflaterHuffman.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/util/zip/DeflaterHuffman.java,v
retrieving revision 1.5
diff -u -r1.5 DeflaterHuffman.java
--- java/util/zip/DeflaterHuffman.java 22 Oct 2004 18:02:06 -0000 1.5
+++ java/util/zip/DeflaterHuffman.java 15 May 2005 20:07:26 -0000
@@ -1,5 +1,5 @@
/* DeflaterHuffman.java --
- Copyright (C) 2001, 2004 Free Software Foundation, Inc.
+ Copyright (C) 2001, 2004, 2005 Free Software Foundation, Inc.
This file is part of GNU Classpath.
@@ -59,7 +59,7 @@
private static final int[] BL_ORDER =
{ 16, 17, 18, 0, 8, 7, 9, 6, 10, 5, 11, 4, 12, 3, 13, 2, 14, 1, 15 };
- private static String bit4Reverse =
+ private static final String bit4Reverse =
"\000\010\004\014\002\012\006\016\001\011\005\015\003\013\007\017";
class Tree {
Index: java/util/zip/DeflaterOutputStream.java
===================================================================
RCS file: /cvsroot/classpath/classpath/java/util/zip/DeflaterOutputStream.java,v
retrieving revision 1.8
diff -u -r1.8 DeflaterOutputStream.java
--- java/util/zip/DeflaterOutputStream.java 20 Oct 2004 08:08:52 -0000 1.8
+++ java/util/zip/DeflaterOutputStream.java 15 May 2005 20:07:11 -0000
@@ -1,5 +1,5 @@
/* DeflaterOutputStream.java - Output filter for compressing.
- Copyright (C) 1999, 2000, 2001, 2004 Free Software Foundation, Inc.
+ Copyright (C) 1999, 2000, 2001, 2004, 2005 Free Software Foundation, Inc.
This file is part of GNU Classpath.
@@ -100,7 +100,7 @@
*/
public DeflaterOutputStream(OutputStream out)
{
- this(out, new Deflater(), 512);
+ this(out, new Deflater(), 4096);
}
/**
@@ -111,7 +111,7 @@
*/
public DeflaterOutputStream(OutputStream out, Deflater defl)
{
- this(out, defl, 512);
+ this(out, defl, 4096);
}
/**
signature.asc
Description: This is a digitally signed message part
_______________________________________________ Classpath mailing list [email protected] http://lists.gnu.org/mailman/listinfo/classpath
