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.
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.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Apr 1 17:24:31 2003