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
>> 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.
>
> 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:
svn_string_from_stream2-v2.patch.txt
[[[
Revv svn_string_from_stream() function and share implementation with
svn_stringbuf_from_stream().
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