[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1707196 - /subversion/trunk/subversion/libsvn_subr/stream.c

From: Stefan Fuhrmann <stefan.fuhrmann_at_wandisco.com>
Date: Wed, 7 Oct 2015 15:46:26 +0200

On Wed, Oct 7, 2015 at 3:21 PM, Julian Foad <julianfoad_at_gmail.com> wrote:

> >>> Julian Foad wrote:
> >>>> [...] I will be
> >>>> interested in reviewing the (single) implementation.
>
> Stefan wrote:
> > [...] Here the final version.
> > If that doesn't work either then I'm done for today.
>
> Stefan: First review comment: You can much more efficiently convert a
> stringbuf to a string (when you own it, like here) using
> svn_stringbuf__morph_into_string().

We could only do that if the stringbuf was allocated in
the result_pool. The original implementation allocates
the read buffer in the scratch_pool and I wanted to
preserve that characteristic. We might want to use
SVN__STREAM_CHUNK_SIZE as LEN_HINT, though
to prevent a few reallocations for larger streams.

> Second review question: did you
> consider whether the second implementation was in fact already better?
> For example, is using _appendbytes better or worse than using
> ensure(size x 2) followed by read()?
>

The appendbytes approach needs a separate buffer to
hold those bytes before they get appended. Ultimately,
it implements the same x2 growth scheme but adds a
copy and a temporary buffer.

-- Stefan^2.
Received on 2015-10-07 15:46:29 CEST

This is an archived mail posted to the Subversion Dev mailing list.