You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2015/09/02 14:33:53 UTC
svn commit: r1700789 - in /subversion/trunk/subversion: include/svn_io.h
libsvn_subr/stream.c svnadmin/svnadmin.c svnfsfs/load-index-cmd.c
tests/libsvn_subr/stream-test.c
Author: stefan2
Date: Wed Sep 2 12:33:52 2015
New Revision: 1700789
URL: http://svn.apache.org/r1700789
Log:
Remove support for mark & seek from "buffered read" stream wrapper as it
was deemed to difficult to handle. See also here:
http://mail-archives.apache.org/mod_mbox/subversion-dev/201509.mbox/%3CCAP_GPNjwhjD1Ds5%2ByYKiqr2NwzbhmnGq%3DqtB8jowBUJty7_Z3Q%40mail.gmail.com%3E
* subversion/include/svn_io.h
(svn_stream_wrap_buffered_read): Remove all references to mark & seek
other than saying we don't support them.
* subversion/libsvn_subr/stream.c
(buffering_stream_wrapper_baton): Remove all elements used for mark & seek.
(buffering_stream_wrapper_mark): Drop.
(read_handler_buffering_wrapper): Simplify as we always exhaust the buffer
and can completely discard it now.
(decrement_mark_count,
mark_handler_buffering_wrapper,
seek_handler_buffering_wrapper,
assert_zero_mark_count): Drop.
(svn_stream_wrap_buffered_read): Update. Allocate the buffer only one and
do that here.
* subversion/svnadmin/svnadmin.c
(subcommand_load_revprops): Update API caller.
* subversion/svnfsfs/load-index-cmd.c
(subcommand__load_index): Update API caller.
* subversion/tests/libsvn_subr/stream-test.c
(test_stream_buffered_wrapper): Update API caller.
Modified:
subversion/trunk/subversion/include/svn_io.h
subversion/trunk/subversion/libsvn_subr/stream.c
subversion/trunk/subversion/svnadmin/svnadmin.c
subversion/trunk/subversion/svnfsfs/load-index-cmd.c
subversion/trunk/subversion/tests/libsvn_subr/stream-test.c
Modified: subversion/trunk/subversion/include/svn_io.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_io.h?rev=1700789&r1=1700788&r2=1700789&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_io.h (original)
+++ subversion/trunk/subversion/include/svn_io.h Wed Sep 2 12:33:52 2015
@@ -1157,29 +1157,16 @@ svn_stream_t *
svn_stream_from_string(const svn_string_t *str,
apr_pool_t *pool);
-/** Return a generic read-only stream that forward a data from INNER
- * but uses buffering and add support for seeking for being more
- * efficient with parsers.
+/** Return a generic read-only stream that forwards data from @a inner but
+ * uses buffering for efficiency. Allocate the stream in @a result_pool.
*
- * Set @a support_seek_to_start to TRUE only if you to be able to seek
- * to the start of stream without prior setting a mark. Since the
- * stream needs to buffer all data since the oldest existing mark,
- * support for @a svn_stream_seek(stream,@c NULL) results in unbounded
- * memory usage. If @a support_seek_to_start is FALSE,
- * @a svn_stream_seek(stream, @c NULL) returns
- * @c SVN_ERR_STREAM_SEEK_NOT_SUPPORTED.
- *
- * Allocate the stream in @a result_pool.
- *
- * @note To keep memory consumption low, make sure to frequently clear
- * the pools that contain marks on the buffered stream. Also, make sure
- * that the total number of marks returns to zero at regular intervals.
+ * @note The returned stream has no support for writing nor mark and seek
+ * even if @a inner does support those functions.
*
* @since New in 1.10.
*/
svn_stream_t *
svn_stream_wrap_buffered_read(svn_stream_t *inner,
- svn_boolean_t support_seek_to_start,
apr_pool_t *result_pool);
/** Return a generic stream which implements buffered reads and writes.
Modified: subversion/trunk/subversion/libsvn_subr/stream.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/stream.c?rev=1700789&r1=1700788&r2=1700789&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/stream.c (original)
+++ subversion/trunk/subversion/libsvn_subr/stream.c Wed Sep 2 12:33:52 2015
@@ -1732,26 +1732,6 @@ struct buffering_stream_wrapper_baton
/* Current read position relative to BUFFER_START. */
apr_size_t buffer_pos;
-
- /* Absolute position within the stream that corresponds to the first
- * byte in BUFFER. */
- svn_filesize_t buffer_start;
-
- /* If TRUE, BUFFER_START must always remain 0.
- * To simplify the code, we initialize MARK_COUNT to 1 in that case, so
- * it can never reach 0. */
- svn_boolean_t support_seek_to_start;
-
- /* Number of marks currently set on this stream. We may only more the
- * BUFFER_START when this is 0. */
- apr_size_t mark_count;
-};
-
-/* svn_stream_mark_t for buffering read stream wrappers. */
-struct buffering_stream_wrapper_mark
-{
- /* Absolute position within the stream. */
- svn_filesize_t pos;
};
/* Implements svn_stream_t.read_fn for buffering read stream wrappers. */
@@ -1767,28 +1747,15 @@ read_handler_buffering_wrapper(void *bat
* So, we only need to replenish our buffers if we ran completely dry. */
if (left_to_read == 0)
{
- /* For the same reason, reading no more than one chunk is fine. */
- apr_size_t count = SVN__STREAM_CHUNK_SIZE;
+ apr_size_t count = btn->buffer->blocksize;
- /* If there is no mark left, we can drop the current buffer contents
- * making room for new data without reallocating the buffer. */
- if (btn->mark_count == 0)
- {
- btn->buffer_start += btn->buffer->len;
- btn->buffer_pos = 0;
- btn->buffer->len = 0;
- }
-
- /* Read from the INNER stream, appending to whatever buffer contents
- * there is currently. */
- svn_stringbuf_ensure(btn->buffer, btn->buffer->len + count);
- SVN_ERR(svn_stream_read2(btn->inner,
- btn->buffer->data + btn->buffer->len,
- &count));
- btn->buffer->len += count;
+ /* Read from the INNER stream. */
+ SVN_ERR(svn_stream_read2(btn->inner, btn->buffer->data, &count));
+ btn->buffer->len = count;
+ btn->buffer_pos = 0;
/* We may now have more data that we could return. */
- left_to_read = btn->buffer->len - btn->buffer_pos;
+ left_to_read = btn->buffer->len;
}
/* Cap the read request to what we can deliver from the buffer. */
@@ -1802,104 +1769,6 @@ read_handler_buffering_wrapper(void *bat
return SVN_NO_ERROR;
}
-/* Pool cleanup callback for stream marks being cleaned up.
- * This will not only do bookkeeping but release buffer space if possible. */
-static apr_status_t
-decrement_mark_count(void *baton)
-{
- struct buffering_stream_wrapper_baton *btn = baton;
-
- /* Bookkeeping. */
- --btn->mark_count;
-
- /* If there are no marks left, we may release / move the buffer.
- *
- * We will do so if we can move the buffer start by at least one chunk
- * (prevent moving data around frequently) and the read pointer is past
- * the middle of the buffer (again, reducing the amount of data copied).
- *
- * After some build-up phase, we will be fluctuating between the minimum
- * needed buffer size and twice that (rounded up to chunk size). The
- * "needed buffer size" is the range covered from the first mark set to
- * the max. read position when the last mark gets cleaned up.
- */
- if ( btn->mark_count == 0
- && btn->buffer_pos > SVN__STREAM_CHUNK_SIZE
- && btn->buffer_pos * 2 > btn->buffer->len)
- {
- /* There is no mark left, i.e. no way to access anything before the
- * current read position. So, make the buffer start at the current
- * read position. */
- svn_stringbuf_remove(btn->buffer, 0, btn->buffer_pos);
-
- btn->buffer_start += btn->buffer_pos;
- btn->buffer_pos = 0;
- }
-
- return APR_SUCCESS;
-}
-
-/* Implements svn_stream_t.mark_fn for buffering read stream wrappers. */
-static svn_error_t *
-mark_handler_buffering_wrapper(void *baton,
- svn_stream_mark_t **mark,
- apr_pool_t *pool)
-{
- struct buffering_stream_wrapper_baton *btn = baton;
- struct buffering_stream_wrapper_mark *stream_mark;
-
- stream_mark = apr_palloc(pool, sizeof(*stream_mark));
- stream_mark->pos = (svn_filesize_t)(btn->buffer_start + btn->buffer_pos);
-
- /* Reference counting: Increment now and schedule automatic the decrement
- * for when the mark is being cleaned up. */
- ++btn->mark_count;
- apr_pool_cleanup_register(pool, baton,
- decrement_mark_count, apr_pool_cleanup_null);
-
- *mark = (svn_stream_mark_t *)stream_mark;
- return SVN_NO_ERROR;
-}
-
-/* Implements svn_stream_t.seek_fn for buffering read stream wrappers. */
-static svn_error_t *
-seek_handler_buffering_wrapper(void *baton,
- const svn_stream_mark_t *mark)
-{
- struct buffering_stream_wrapper_baton *btn = baton;
-
- if (mark != NULL)
- {
- const struct buffering_stream_wrapper_mark *stream_mark
- = (const struct buffering_stream_wrapper_mark *)mark;
-
- /* Position to move to, relative to the start of the buffer.
- * The casts here are safe due to svn_filesize_t being 64 bits. */
- svn_filesize_t pos = stream_mark->pos - btn->buffer_start;
-
- /* The mark must point to somewhere inside our buffered location or
- * immediately at the upper end of the buffer. */
- SVN_ERR_ASSERT(pos >= 0 && pos <= (svn_filesize_t)btn->buffer->len);
- btn->buffer_pos = (apr_size_t)pos;
- }
- else if (btn->support_seek_to_start)
- {
- /* To support this feature, we locked the buffer at position 0.
- * Make sure that this is still the case. */
- SVN_ERR_ASSERT(btn->buffer_start == 0);
-
- /* Next read starts at the begin of the stream again. */
- btn->buffer_pos = 0;
- }
- else
- {
- return svn_error_create(SVN_ERR_STREAM_SEEK_NOT_SUPPORTED, NULL,
- _("Can't seek to begin of stream."));
- }
-
- return SVN_NO_ERROR;
-}
-
/* Implements svn_stream_t.data_available_fn for buffering read stream
* wrappers. */
static svn_error_t *
@@ -1929,23 +1798,8 @@ is_buffered_handler_buffering_wrapper(vo
return TRUE;
}
-/* Pool cleanup callback checking that there are no more user-created
- * marks on that buffering_stream_wrapper_baton BATON. If there were,
- * they might cause memory corruption in their cleanup code. */
-static apr_status_t
-assert_zero_mark_count(void *baton)
-{
- struct buffering_stream_wrapper_baton *btn = baton;
-
- apr_size_t expected_mark_count = btn->support_seek_to_start ? 1 : 0;
- SVN_ERR_ASSERT_NO_RETURN(btn->mark_count == expected_mark_count);
-
- return APR_SUCCESS;
-}
-
svn_stream_t *
svn_stream_wrap_buffered_read(svn_stream_t *inner,
- svn_boolean_t support_seek_to_start,
apr_pool_t *result_pool)
{
svn_stream_t *stream;
@@ -1955,28 +1809,17 @@ svn_stream_wrap_buffered_read(svn_stream
* The buffer is empty and we are at position 0. */
baton = apr_pcalloc(result_pool, sizeof(*baton));
baton->inner = inner;
- baton->buffer = svn_stringbuf_create_empty(result_pool);
+ baton->buffer = svn_stringbuf_create_ensure(SVN__STREAM_CHUNK_SIZE,
+ result_pool);
baton->buffer_pos = 0;
- baton->buffer_start = 0;
- baton->support_seek_to_start = support_seek_to_start;
-
- /* If seek(NULL) is supported, lock the buffer like a mark at position 0
- * would. */
- baton->mark_count = support_seek_to_start ? 1 : 0;
/* Create the wrapper stream object and set up the vtable. */
stream = svn_stream_create(baton, result_pool);
svn_stream_set_read2(stream, read_handler_buffering_wrapper, NULL);
- svn_stream_set_mark(stream, mark_handler_buffering_wrapper);
- svn_stream_set_seek(stream, seek_handler_buffering_wrapper);
svn_stream_set_data_available(stream,
data_available_handler_buffering_wrapper);
svn_stream__set_is_buffered(stream, is_buffered_handler_buffering_wrapper);
- /* Assert that no marks get cleaned up after the stream. */
- apr_pool_cleanup_register(result_pool, baton,
- assert_zero_mark_count, apr_pool_cleanup_null);
-
return stream;
}
Modified: subversion/trunk/subversion/svnadmin/svnadmin.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnadmin/svnadmin.c?rev=1700789&r1=1700788&r2=1700789&view=diff
==============================================================================
--- subversion/trunk/subversion/svnadmin/svnadmin.c (original)
+++ subversion/trunk/subversion/svnadmin/svnadmin.c Wed Sep 2 12:33:52 2015
@@ -1548,7 +1548,7 @@ subcommand_load_revprops(apr_getopt_t *o
/* Read the stream from STDIN. Users can redirect a file. */
SVN_ERR(svn_stream_for_stdin(&stdin_stream, pool));
- stdin_stream = svn_stream_wrap_buffered_read(stdin_stream, FALSE, pool);
+ stdin_stream = svn_stream_wrap_buffered_read(stdin_stream, pool);
/* Progress feedback goes to STDOUT, unless they asked to suppress it. */
if (! opt_state->quiet)
Modified: subversion/trunk/subversion/svnfsfs/load-index-cmd.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnfsfs/load-index-cmd.c?rev=1700789&r1=1700788&r2=1700789&view=diff
==============================================================================
--- subversion/trunk/subversion/svnfsfs/load-index-cmd.c (original)
+++ subversion/trunk/subversion/svnfsfs/load-index-cmd.c Wed Sep 2 12:33:52 2015
@@ -187,7 +187,7 @@ subcommand__load_index(apr_getopt_t *os,
svn_stream_t *input;
SVN_ERR(svn_stream_for_stdin(&input, pool));
- input = svn_stream_wrap_buffered_read(input, FALSE, pool);
+ input = svn_stream_wrap_buffered_read(input, pool);
SVN_ERR(load_index(opt_state->repository_path, input, pool));
return SVN_NO_ERROR;
Modified: subversion/trunk/subversion/tests/libsvn_subr/stream-test.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/tests/libsvn_subr/stream-test.c?rev=1700789&r1=1700788&r2=1700789&view=diff
==============================================================================
--- subversion/trunk/subversion/tests/libsvn_subr/stream-test.c (original)
+++ subversion/trunk/subversion/tests/libsvn_subr/stream-test.c Wed Sep 2 12:33:52 2015
@@ -908,7 +908,7 @@ test_stream_buffered_wrapper(apr_pool_t
* This requires multiple reads per line while readline will hold marks
* etc. */
svn_stream_t *stream = create_test_read_stream(stream_length, 19, pool);
- stream = svn_stream_wrap_buffered_read(stream, FALSE, pool);
+ stream = svn_stream_wrap_buffered_read(stream, pool);
/* We told the stream not to supports seeking to the start. */
SVN_TEST_ASSERT_ERROR(svn_stream_seek(stream, NULL),
@@ -933,11 +933,6 @@ test_stream_buffered_wrapper(apr_pool_t
read += line->len + 1;
}
- /* Test a stream wrapper that supports seeking to the start. */
- stream = create_test_read_stream(stream_length, 19, pool);
- stream = svn_stream_wrap_buffered_read(stream, TRUE, pool);
- SVN_ERR(svn_stream_seek(stream, NULL));
-
return SVN_NO_ERROR;
}