Re: [PATCH] Compressed streams (take 2)

Eric Dorland <eric.dorland@mail.mcgill.ca> writes:

> It was basically arbitrary. I picked something that wasn't too small
> and that would likely be the size of a page, or some constant multiple
> of the size of a page.

That's the sort of information that should go in a comment beside the
constant.

> I'm sure different sizes would yield different
> performance characteristics in different situations (eg a bigger
> buffer would be better for larger reads, but penalize smaller ones). I
> don't think it really matters too much, since I think the IO system
> under Linux and other Unices would cache things enough to make it
> pretty irrelevant what I choose (as long as it's not too insane).

APR will also provides buffering of files if requested.

> I could even make the buffer dynamic in size, with some heuristic
> based on the amount of data the user wants to read it.

I'm not sure a heuristic is a good idea, mainly because I don't know
how to evaluate it. What about a buffer size argument to
svn_stream_compressed? Is that a good idea? Perhaps with a #define
(your 4096) for a default value.

> 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.

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org