On 22 October 2015, Ivan Zhakov wrote:
> On 19 October 2015, Ivan Zhakov wrote:
>> [[[
>> Revv svn_string_from_stream() function and share implementation with
>> svn_stringbuf_from_stream().
>>
>> Suggested by: julianf
[...]
>> ]]]
>>
> Committed in r1710065.
>
>> [[[
>> * 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.
>> ]]]
>>
> Committed in r1710066.
OK, looks good. (I haven't reviewed the implementation in detail, yet.)
Two more things:
1. The doc string should explain len_hint in the caller's terms. The
doc strings also differ in other unnecessary respects. I propose the
attached documentation patch.
2. The _string and _stringbuf function declarations still differ in
that only svn_string_from_stream2() takes a scratch pool. The current
implementation doesn't use it. We should unify them. Which is best,
taking a scratch pool or not?
Stefan Fuhrmann wrote earlier:
> Maybe get rid of the scratch_pool because we won't use it nor do any of the
> current
> callers care.
In the absence of any other argument, I would accept that argument and
remove the scratch pool.
- Julian
Improve the documentation of svn_stringbuf[buf]_from_stream(), especially in
describing the length hint parameter and in documenting the two similar
functions the same way.
Also rename the output parameter of one of them for consistency with the
other.
* subversion/include/svn_io.h
(svn_stringbuf_from_stream): Rename the output parameter 'str' to 'result'.
Improve the documentation.
(svn_string_from_stream2): Improve the documentation.
* subversion/libsvn_subr/stream.c
(svn_stringbuf_from_stream): Rename the output parameter 'str' to 'result'.
--This line, and those below, will be ignored--
Index: subversion/include/svn_io.h
===================================================================
--- subversion/include/svn_io.h (revision 1710133)
+++ subversion/include/svn_io.h (working copy)
@@ -1144,22 +1144,36 @@ svn_stream_for_stderr(svn_stream_t **err
* when @a pool is cleared or destroyed.
*/
svn_error_t *
svn_stream_for_stdout(svn_stream_t **out,
apr_pool_t *pool);
-/** Set @a *str to a string buffer allocated in @a result_pool that contains
- * all data from the current position in @a stream to its end. @a len_hint
- * specifies the initial capacity of the string buffer and may be 0. The
- * buffer gets automatically resized to fit the actual amount of data being
- * read from @a stream.
+/** Read the contents of @a stream into memory, from its current position
+ * to its end, returning the data in @a *result. The stream will be closed
+ * when it has been successfully and completely read.
+ *
+ * @a len_hint gives a hint about the expected length, in bytes, of the
+ * actual data that will be read from the stream. It may be 0, meaning no
+ * hint is being provided. Efficiency in time and/or in space may be
+ * better (and in general will not be worse) when the actual data length
+ * is equal or approximately equal to the length hint.
+ *
+ * The present implementation:
+ * - uses an effective hint of 64 bytes when a lower value or zero is
+ * specified;
+ * - uses minimal time and space when the actual length is less than or
+ * equal to the effective hint;
+ * - uses around 1.5x to 2x the minimal time and space when the actual
+ * length is greater than the effective hint.
+ *
+ * The returned memory is allocated in @a result_pool.
*
* @since New in 1.9.
*/
svn_error_t *
-svn_stringbuf_from_stream(svn_stringbuf_t **str,
+svn_stringbuf_from_stream(svn_stringbuf_t **result,
svn_stream_t *stream,
apr_size_t len_hint,
apr_pool_t *result_pool);
/** Return a generic stream connected to stringbuf @a str. Allocate the
* stream in @a pool.
@@ -1517,25 +1531,33 @@ svn_error_t *
svn_stream_contents_same(svn_boolean_t *same,
svn_stream_t *stream1,
svn_stream_t *stream2,
apr_pool_t *pool);
-/** Read the contents of @a stream into memory, returning the data in
- * @a result. The stream will be closed when it has been successfully and
- * completely read.
- *
- * @a len_hint specifies the initial capacity of the string buffer and
- * may be 0.
+/** Read the contents of @a stream into memory, from its current position
+ * to its end, returning the data in @a *result. The stream will be closed
+ * when it has been successfully and completely read.
+ *
+ * @a len_hint gives a hint about the expected length, in bytes, of the
+ * actual data that will be read from the stream. It may be 0, meaning no
+ * hint is being provided. Efficiency in time and/or in space may be
+ * better (and in general will not be worse) when the actual data length
+ * is equal or approximately equal to the length hint.
+ *
+ * The present implementation:
+ * - uses an effective hint of 64 bytes when a lower value or zero is
+ * specified;
+ * - uses minimal time and space when the actual length is less than or
+ * equal to the effective hint;
+ * - uses around 1.5x to 2x the minimal time and space when the actual
+ * length is greater than the effective hint.
*
* The returned memory is allocated in @a result_pool, and any temporary
* allocations are performed in @a scratch_pool.
*
- * @note due to memory pseudo-reallocation behavior (due to pools), this
- * can be a memory-intensive operation for large files.
- *
* @since New in 1.10
*/
svn_error_t *
svn_string_from_stream2(svn_string_t **result,
svn_stream_t *stream,
apr_size_t len_hint,
Index: subversion/libsvn_subr/stream.c
===================================================================
--- subversion/libsvn_subr/stream.c (revision 1710133)
+++ subversion/libsvn_subr/stream.c (working copy)
@@ -1479,13 +1479,13 @@ svn_stream_checksummed2(svn_stream_t *st
return s;
}
/* Miscellaneous stream functions. */
svn_error_t *
-svn_stringbuf_from_stream(svn_stringbuf_t **str,
+svn_stringbuf_from_stream(svn_stringbuf_t **result,
svn_stream_t *stream,
apr_size_t len_hint,
apr_pool_t *result_pool)
{
#define MIN_READ_SIZE 64
svn_stringbuf_t *text
@@ -1505,13 +1505,13 @@ svn_stringbuf_from_stream(svn_stringbuf_
if (text->blocksize < text->len + MIN_READ_SIZE)
svn_stringbuf_ensure(text, text->blocksize * 2);
}
text->data[text->len] = '\0';
- *str = text;
+ *result = text;
return SVN_NO_ERROR;
}
struct stringbuf_stream_baton
{
