[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: Ivan Zhakov <ivan_at_visualsvn.com>
Date: Mon, 19 Oct 2015 19:22:57 +0300

On 7 October 2015 at 18:10, Julian Foad <julianfoad_at_gmail.com> wrote:
> Stefan Fuhrmann wrote:
>> Julian Foad wrote:
>>> 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
>> 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.
> OK, that's totally clear, and after your comment above about using
> SVN__STREAM_CHUNK_SIZE as LEN_HINT, it sounds like you have considered
> the differences. That was all I wanted to hear. Thanks.
Hi Julian,

I propose attached two patches as follow-up to r1707196:
Revv svn_string_from_stream() function and share implementation with

Suggested by: julianf

* subversion/include/svn_io.h
  (svn_string_from_stream2): New.
  (svn_string_from_stream): Deprecate.

* subversion/libsvn_subr/stream.c
  (svn_string_from_stream2): Revv from svn_string_from_stream(): add LEN_HINT
   argument. Use svn_stringbuf_from_stream() as implementation.

* subversion/libsvn_subr/deprecated.c
  (svn_string_from_stream): Call svn_string_from_stream2() with LEN_HINT=0.

* subversion/libsvn_fs_x/reps.c
* subversion/libsvn_wc/old-and-busted.c
* tools/dev/x509-parser.c
  (svn_fs_x__reps_add_base, svn_wc__read_entries_old,
   get_der_cert_from_stream): Use svn_string_from_stream2() with
   LEN_HINT=SVN__STREAM_CHUNK_SIZE. It doesn't increase memory usage because
   we use same pool for SCRATCH and RESULT pool.

And svn_stringbuf_from_stream-v1.patch.txt
* subversion/libsvn_subr/stream.c
  (svn_stringbuf_from_stream): Optimize memory usage a bit and avoid
   svn_stream_read_full() call once we got partial read.

Ivan Zhakov

Received on 2015-10-19 18:23:25 CEST

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