Re: [PATCH] Compressed streams (take 2)

* Philip Martin (philip@codematters.co.uk) wrote:
> Greg Stein <gstein@lyra.org> writes:
>
> > On Tue, Apr 01, 2003 at 01:29:42AM +0100, Philip Martin wrote:
> > > Eric Dorland <eric.dorland@mail.mcgill.ca> writes:
> > >...
> > > > It's probably
> > > > not worth it though. But hey, if it gets my patch in, I'll do it :)
> > >
> > > I'm not trying to be awkward, I'm just trying to understand the patch
> > > without going to the trouble of reading the zlib documentation.
> >
> > How about a slight change in approach here?
> >
> > * is the concept useful and applicable? do we really want this feature?
> > * is the current code "close enough"? can the tweaks be made to the code
> > after it has been applied?
> >
> > My concern here is that if we ask patch submitters to tweak a nit here, and
> > tweak a nit there, each coming in at different times as different reviewers
> > find time to look at the code, that eventually the submitter will just throw
> > their hands up in the air and go away.
> >
> > Now, I'm not saying "apply bad patches and fix them later." But I am saying,
> > "can we alter the buffer size after the code is in?" or "can the data
> > generation occur after the code is in?" It would seem that neither of those
> > is fundamental to having the code added such that people can begin to
> > exercise it in new ways.
>
> I do think this patch, or something very like it, should be applied. I
> hope Eric doesn't get put off by my questions.
>
> What concerns me is the interface. This stream wraps some other
> stream, which may or may not have a buffering policy. The streams are
> generally built onto APR files, which do have a buffering policy. It
> is possible that some other stream will be used to wrap the zlib
> stream. Now while APR provides an interface to enable or disable
> buffering, this zlib stream hard codes the decision. It may well be
> that zlib makes some form of buffering unavoidable, but I think it is
> reasonable to ask if the interface is correct. If we blindly put
> buffering everywhere it's possible that we will reduce performance
> rather than increase it.

It is somewhat unavoidable. For example, if the user does a read of 10
bytes, I have no way of knowing how man compressed bytes I will need
to get those 10 bytes. It could be more, or it could be less, I have
no way of knowing. I just have to hand inflate bytes until it gives me
back the 10 bytes I need. If we don't want to do buffering, I can read
a single byte at a time and pass each one to inflate. I can't really
imagine a situation where that could be faster though.
 
> That said, I think will apply Eric's latest patch. I might add a
> comment to the header file mentioning that the compressed stream uses
> a buffer. Although this exposes the implementation I think it is
> something users of the stream need to know.

Thanks, that's perfectly fine.

-- 
Eric Dorland <eric.dorland@mail.mcgill.ca>
ICQ: #61138586, Jabber: hooty@jabber.com
1024D/16D970C6 097C 4861 9934 27A0 8E1C  2B0A 61E9 8ECF 16D9 70C6
-----BEGIN GEEK CODE BLOCK-----
Version: 3.12
GCS d- s++: a-- C+++ UL+++ P++ L++ E++ W++ N+ o K- w+ 
O? M++ V-- PS+ PE Y+ PGP++ t++ 5++ X+ R tv++ b+++ DI+ D+ 
G e h! r- y+ 
------END GEEK CODE BLOCK------