Early version of striding Deflater

[Dave Bristor]

Hi Clemens,

Thanks for the code drop!  I was not yet able to spend much Quality Time with 
it.  At a glance, it seems like the fix might work.  The general ideas seem 
sound, and I appreciate your effort in addressing it.  So please don't take 
the feedback below as anything but encouragement to help get this bug fixed!

That said...it is a low priority bug (for us, anyway; a P4 out of 5) and we 
have bigger fish to fry.  I'll attend to it as time permits... Then too, there 
are some issues to resolve re the provided code.  I know it's an "Early 
version", but some changes to it would make it easier to examine.

For example, Deflater.java was completely reformatted.  When diffing the code, 
every changeof indentation, javadoc removal, spaces to tabs, moving of {, etc. 
shows up.  We don't allow such changes into the JDK sources.  Spaces only, and 
despite whatever awful "standards" (or not!) already in use in the file, 
please stick to them.  Make every change be the best/smallest one which 
directly addresses the bug.  This makes it easier for all concerned to examine 
the relevant changes.  There are similar issues in Deflater.c.

The files provided the lack the GNU copyright file headers.  My guess is that 
they originated in the src.zip of a binary distribution.  Regardless of their 
source, could you please instead use files from mercurial repository at 
hg.openjdk.java.net/jdk7/tl?

The above will make it easier to review future changes to the files.  Enough 
of that boring stuff, here's some feedback on "interesting" part of the changes!

In Deflater.java, new method rangeCheck() is used in a couple of places, but 
it is not an adequate replacement for the code previously in 
Deflater.setDictionary(); it incompatibly changes the error condition checking 
semantics (we can't omit the check on strm).  We strive to not introduce 
incompatible changes, even small ones like this.

Another incompatible change is to the semantics of Deflater.deflate()...I 
think.  In Deflater.c, it seems that deflateBytes will use setParams if 
necessary and then compress...I reference your email about 6539727 in this 
regard (You are completely right about that being a non-bug, BTW and I'll 
update it shortly: thanks!)  I think your solution would have been the Right 
Thing to do way back when, but we don't want to make an incompatible change 
now.  (I haven't reviewed this thoroughly enough; see my notes re formatting & 
priorities above.)

With a change of this sort, we really do need tests along with a fix.  Have 
you started writing any test cases?

Finally, it seems that this solution obviates the need for the striding in 
DeflaterOutputStream...IIRC, that is some of the original motivation for this 
work.  If you have suggested changes to that class as well, please include them.

I appreciate the work you've put in, and again, I hope to not dissuade you. 
But we have certain standards to which we must adhere, and it's a not a very 
high priority for us now, so we have to minimize the time we spend on it.

Thanks,
	Dave

Clemens Eisserer wrote:
> Hello again,
> 
> I've finished an early version of the java.util.zip.Deflater
> implementation that uses striding. Its in an early stage and quite
> likely will be buggy.
> It passes FlaterTest and a simple test written by myself, but maybe
> acts differently in corner-cases.
> 
> I would be happy to receive some comments as well as criticism ;)
> 
> lg Clemens
> 
> PS: Sorry for the traffic lately.