4206909 - adding Z_SYNC_FLUSH support to deflaters

[Xueming Shen]

I sometime too feels the same frustration when get kicked by the 
compatibility, especially if you have to
back out hundreds of lines of code just because it is "incompatible". I 
guess someone said it right a while
ago,  the compatibility sucks, but yes, MOST end user/developer like it.

I disagree with the statement that "we are not fixing it". As I said in 
my last email, the proposed change does
address the issue and provide a reasonable solution to the problem that 
can't not be solved with the existing
APIs.  My understanding of  the key issue here is that the existing 
DeflaterOutputStream and Deflater do not
provide the method(s) to sync_flush the underlying deflater, which is a 
must for some use scenario. I believe
most developers should be happy as long as such methods are provided, it 
is NOT necessary to be the flush().

Yes, the justification for me to go with the conservative approach here 
is exactly the "behavior has been in this
way for 10 years". Let me share my consideration here again.

For existing running application, the DOS.flush() has not been working 
the way someone might expect for
decade, so I don't think there is any application is sitting tightly and 
patiently over there for years to just
wait for us to "fix" the DOS.flush() behavior and  their app will then 
magically work as "expected". They
either go with the ugly workaround or go use jzlib.

For developer that are waiting this to be fix so that they can simply 
use the default zlib included in java in their
application, the proposed solution just works fine, use the flush(int) 
when needed, they have to explicitly invoke
A flush method anyway, why it can not be the flush(int) (or say, why it 
must be the flush()).

The only benefit of having a sync_flush flush() in DOS is that you don't 
have to override the default flush() when
you have to pass the DOS around and "flush()" will be invoked 
"indirectly" by other part/layer of the code. The
price to pay for this "convenience" is the incompatibility. The 
applications that works for years might suddenly
experience the "regression" when migrated to JDK7. We DONT know if there 
are really such applications there
and how important they are until we hit it real world. At this point, 
the merit of doing this seems not worth it.

I can see other benefit of doing this. If we change the behavior of 
DOS.flush() in JDK7 and do not see lots
of "regression" complain over there, it will be then easy to backport 
part of the change (means the DOS.flush()
only) into 6 releases.

I agree it might be a better to have some thing in the DOS api to 
explain the situation, how about  to have
a note like below to add into the flush(int) API doc.

<p>Note: The inherited flush() method only flushes the output stream 
without flushing the compressor(), it
should be overridden to do flush(Deflater.SYNC_FLUSH) or 
flush(Deflate.FULL_FLUSH) if desirable.


sherman






Brandon Long wrote:
> I really don't understand this.  I think just about everyone would
> expect that flush on an output stream means send all the data I've
> written.  There are a large number of people who have stumbled on this
> bug and several others, and the fact that it wasn't fixed when it was
> discovered 10 years ago is justification for not fixing it now?
>
> I imagine a lot of people will stumble on this in the future, instead of
> it just working as expected.
>
> This is up against some unknown fraction of people who are for some
> reason happy with the current broken behavior, and would dislike paying
> a 10-50% penalty in compression because their code calls flush when they
> don't actually need to?
>
> At the very least, this misfeature needs to be called out in the
> documentation for the DeflaterOutputStream class.
>
> Brandon
>
> On 09/09/09 Xueming Shen uttered the following other thing:
>   
>> Martin, Brandon, Alan
>>
>> I still believe the best choice at this moment is to be a little more  
>> conservative, given the DOS.flush() has been
>> working in this behavior for decade. The proposed methods are good  
>> enough to serve the functionality required,
>> I don't think it is worth breaking the compatibility in this case simply  
>> because "it would be better...". We definitely
>> can consider to change the position should the feedback show this is not  
>> the case.
>>
>> I will start the CCC if you guys are OK with the latest wording. The doc  
>> has been changed "slightly" compared to
>> last version, thanks for the comment from all of you.
>>
>> http://cr.openjdk.java.net/~sherman/zipflush/webrev
>>
>> Sherman
>>     
>
>