4206909 - adding Z_SYNC_FLUSH support to deflaters
Xueming.Shen at Sun.COM
Wed Sep 9 21:19:30 UTC 2009
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.
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.
> 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.
More information about the core-libs-dev