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/08/31 21:16:07 UTC

svn commit: r1700305 - in /subversion/trunk/subversion: include/svn_io.h libsvn_subr/stream.c libsvn_subr/subst.c

Author: stefan2
Date: Mon Aug 31 19:16:06 2015
New Revision: 1700305

URL: http://svn.apache.org/r1700305
Log:
Add a function to destroy / remove a mark from a stream.

For some streams, a mark is a relatively expensive object.  Merely relying
on timely pool cleanups is not always enough because our pool usage pattern
often relies on the caller to clean up pools in a timely manner.  Having
explicit control over the lifetime of stream marks similar to what we have
with file objects will improve ease of use.

In this patch, only wrapping streams and those with "heavy" mark objects
implement svn_stream_remove_mark().  For all other streams that support
svn_stream_mark() it defaults to a no-op.

We use the new API in svn_stream_readline() to prevent long-living marks
on the new buffering read stream wrapper.  This, in turn, limits the buffer
size of that stream to 2x SVN__STREAM_CHUNK_SIZE in all non-degenerate
cases.

* subversion/include/svn_io.h
  (svn_stream_remove_mark_fn_t,
   svn_stream_set_remove_mark,
   svn_stream_remove_mark): Definition trio for the new public stream API.

* subversion/libsvn_subr/stream.c
  (svn_stream_t): Add a  function table entry for the new function. 
  (svn_stream_set_remove_mark): Implement new public API.
  (svn_stream_remove_mark): Same. Make it a no-op by default.

  (stream_readline_chunky): Use the new API to release stream marks asap.

  (remove_mark_handler_disown,
   svn_stream_disown): Explicitly implement the new API as forwarding to
                       the wrapped stream.
  (buffering_stream_wrapper_mark): Extend the mark object such that we
                                   can call the cleanup explicitly.
  (mark_handler_buffering_wrapper): Update mark constructor.
  (remove_mark_handler_buffering_wrapper,
   svn_stream_wrap_buffered_read): Explicitly implement the new API with the
                                   same effect as an explicit pool cleanup.
  (seek_handler_lazyopen,
   svn_stream_lazyopen_create): Explicitly implement the new API as
                                forwarding to the wrapped stream.

* subversion/libsvn_subr/subst.c
  (translated_stream_seek,
   stream_translated): Same.

Modified:
    subversion/trunk/subversion/include/svn_io.h
    subversion/trunk/subversion/libsvn_subr/stream.c
    subversion/trunk/subversion/libsvn_subr/subst.c

Modified: subversion/trunk/subversion/include/svn_io.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_io.h?rev=1700305&r1=1700304&r2=1700305&view=diff
==============================================================================
--- subversion/trunk/subversion/include/svn_io.h (original)
+++ subversion/trunk/subversion/include/svn_io.h Mon Aug 31 19:16:06 2015
@@ -910,6 +910,14 @@ typedef svn_error_t *(*svn_stream_mark_f
 typedef svn_error_t *(*svn_stream_seek_fn_t)(void *baton,
                                              const svn_stream_mark_t *mark);
 
+/** Handler function for removing a mark from a generic stream.
+ * @see svn_stream_t and svn_stream_remove_mark().
+ *
+ * @since New in 1.10.
+ */
+typedef svn_error_t *(*svn_stream_remove_mark_fn_t)(void *baton,
+                                                    svn_stream_mark_t *mark);
+
 /** Poll handler for generic streams that support incomplete reads, @see
  * svn_stream_t and svn_stream_data_available().
  *
@@ -984,6 +992,14 @@ void
 svn_stream_set_seek(svn_stream_t *stream,
                     svn_stream_seek_fn_t seek_fn);
 
+/** Set @a stream's mark removal function to @a remove_mark_fn
+ *
+ * @since New in 1.10.
+ */
+void
+svn_stream_set_remove_mark(svn_stream_t *stream,
+                           svn_stream_remove_mark_fn_t remove_mark_fn);
+
 /** Set @a stream's data available function to @a data_available_fn
  *
  * @since New in 1.9.
@@ -1369,6 +1385,18 @@ svn_stream_mark(svn_stream_t *stream,
 svn_error_t *
 svn_stream_seek(svn_stream_t *stream, const svn_stream_mark_t *mark);
 
+/** Remove a @a mark from a generic @a stream.  Using this @a mark with
+ * svn_stream_seek() afterwards results in undefined behavior.
+ *
+ * This function returns the #SVN_ERR_STREAM_SEEK_NOT_SUPPORTED error
+ * if the stream doesn't implement seeking.
+ *
+ * @see svn_stream_mark()
+ * @since New in 1.10.
+ */
+svn_error_t *
+svn_stream_remove_mark(svn_stream_t *stream, svn_stream_mark_t *mark);
+
 /** When a stream supports polling for available data, obtain a boolean
  * indicating whether data is waiting to be read. If the stream doesn't
  * support polling this function returns a #SVN_ERR_STREAM_NOT_SUPPORTED

Modified: subversion/trunk/subversion/libsvn_subr/stream.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/stream.c?rev=1700305&r1=1700304&r2=1700305&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/stream.c (original)
+++ subversion/trunk/subversion/libsvn_subr/stream.c Mon Aug 31 19:16:06 2015
@@ -59,6 +59,7 @@ struct svn_stream_t {
   svn_close_fn_t close_fn;
   svn_stream_mark_fn_t mark_fn;
   svn_stream_seek_fn_t seek_fn;
+  svn_stream_remove_mark_fn_t remove_mark_fn;
   svn_stream_data_available_fn_t data_available_fn;
   svn_stream__is_buffered_fn_t is_buffered_fn;
   apr_file_t *file; /* Maybe NULL */
@@ -131,6 +132,13 @@ svn_stream_set_seek(svn_stream_t *stream
 }
 
 void
+svn_stream_set_remove_mark(svn_stream_t *stream,
+                           svn_stream_remove_mark_fn_t remove_mark_fn)
+{
+  stream->remove_mark_fn = remove_mark_fn;
+}
+
+void
 svn_stream_set_data_available(svn_stream_t *stream,
                               svn_stream_data_available_fn_t data_available_fn)
 {
@@ -251,6 +259,22 @@ svn_stream_seek(svn_stream_t *stream, co
 }
 
 svn_error_t *
+svn_stream_remove_mark(svn_stream_t *stream, svn_stream_mark_t *mark)
+{
+  if (stream->remove_mark_fn == NULL)
+    {
+      if (stream->mark_fn == NULL)
+        return svn_error_create(SVN_ERR_STREAM_SEEK_NOT_SUPPORTED, NULL,
+			        NULL);
+
+       /* This stream simply does not care about existing marks. */
+       return SVN_NO_ERROR;
+    }
+
+  return svn_error_trace(stream->remove_mark_fn(stream->baton, mark));
+}
+
+svn_error_t *
 svn_stream_data_available(svn_stream_t *stream,
                           svn_boolean_t *data_available)
 {
@@ -476,7 +500,8 @@ stream_readline_chunky(svn_stringbuf_t *
   /* Move the stream read pointer to the first position behind the EOL.
    */
   SVN_ERR(svn_stream_seek(stream, mark));
-  return svn_error_trace(svn_stream_skip(stream, total_parsed));
+  SVN_ERR(svn_stream_skip(stream, total_parsed));
+  return svn_error_trace(svn_stream_remove_mark(stream, mark));
 }
 
 /* Guts of svn_stream_readline().
@@ -779,6 +804,12 @@ seek_handler_disown(void *baton, const s
 }
 
 static svn_error_t *
+remove_mark_handler_disown(void *baton, svn_stream_mark_t *mark)
+{
+  return svn_error_trace(svn_stream_remove_mark(baton, mark));
+}
+
+static svn_error_t *
 data_available_disown(void *baton, svn_boolean_t *data_available)
 {
   return svn_error_trace(svn_stream_data_available(baton, data_available));
@@ -800,6 +831,7 @@ svn_stream_disown(svn_stream_t *stream,
   svn_stream_set_write(s, write_handler_disown);
   svn_stream_set_mark(s, mark_handler_disown);
   svn_stream_set_seek(s, seek_handler_disown);
+  svn_stream_set_remove_mark(s, remove_mark_handler_disown);
   svn_stream_set_data_available(s, data_available_disown);
   svn_stream__set_is_buffered(s, is_buffered_handler_disown);
 
@@ -1752,6 +1784,9 @@ struct buffering_stream_wrapper_mark
 {
     /* Absolute position within the stream. */
     svn_filesize_t pos;
+
+    /* The pool that has the cleanup function registered for this mark. */
+    apr_pool_t *pool;
 };
 
 /* Implements svn_stream_t.read_fn for buffering read stream wrappers. */
@@ -1850,6 +1885,7 @@ mark_handler_buffering_wrapper(void *bat
 
   stream_mark = apr_palloc(pool, sizeof(*stream_mark));
   stream_mark->pos = (svn_filesize_t)(btn->buffer_start + btn->buffer_pos);
+  stream_mark->pool = pool;
 
   /* Reference counting: Increment now and schedule automatic the decrement
    * for when the mark is being cleaned up. */
@@ -1900,6 +1936,25 @@ seek_handler_buffering_wrapper(void *bat
   return SVN_NO_ERROR;
 }
 
+/* Implements svn_stream_t.remove_mark_fn for buffering read stream wrappers.
+ */
+static svn_error_t *
+remove_mark_handler_buffering_wrapper(void *baton,
+                                      svn_stream_mark_t *mark)
+{
+  struct buffering_stream_wrapper_mark *stream_mark
+    = (struct buffering_stream_wrapper_mark *)mark;
+
+  /* Bookkeeping. */
+  apr_pool_cleanup_run(stream_mark->pool, baton, decrement_mark_count);
+
+  /* Invalidate the mark object so we get an error when trying to use it. */
+  stream_mark->pos = -1;
+  stream_mark->pool = NULL;
+
+  return SVN_NO_ERROR;
+}
+
 /* Implements svn_stream_t.data_available_fn for buffering read stream
  * wrappers. */
 static svn_error_t *
@@ -1969,6 +2024,7 @@ svn_stream_wrap_buffered_read(svn_stream
   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_remove_mark(stream, remove_mark_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);
@@ -2213,6 +2269,19 @@ seek_handler_lazyopen(void *baton,
   return SVN_NO_ERROR;
 }
 
+/* Implements svn_stream_remove_mark_fn_t */
+static svn_error_t *
+remove_mark_handler_lazyopen(void *baton,
+                             svn_stream_mark_t *mark)
+{
+  lazyopen_baton_t *b = baton;
+
+  SVN_ERR(lazyopen_if_unopened(b));
+  SVN_ERR(svn_stream_remove_mark(b->real_stream, mark));
+
+  return SVN_NO_ERROR;
+}
+
 static svn_error_t *
 data_available_handler_lazyopen(void *baton,
                                 svn_boolean_t *data_available)
@@ -2260,6 +2329,7 @@ svn_stream_lazyopen_create(svn_stream_la
   svn_stream_set_close(stream, close_handler_lazyopen);
   svn_stream_set_mark(stream, mark_handler_lazyopen);
   svn_stream_set_seek(stream, seek_handler_lazyopen);
+  svn_stream_set_remove_mark(stream, remove_mark_handler_lazyopen);
   svn_stream_set_data_available(stream, data_available_handler_lazyopen);
   svn_stream__set_is_buffered(stream, is_buffered_lazyopen);
 

Modified: subversion/trunk/subversion/libsvn_subr/subst.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/subst.c?rev=1700305&r1=1700304&r2=1700305&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/subst.c (original)
+++ subversion/trunk/subversion/libsvn_subr/subst.c Mon Aug 31 19:16:06 2015
@@ -1431,6 +1431,18 @@ translated_stream_seek(void *baton, cons
   return SVN_NO_ERROR;
 }
 
+/* Implements svn_stream_remove_mark_fn_t. */
+static svn_error_t *
+translated_stream_remove_mark(void *baton, svn_stream_mark_t *mark)
+{
+  struct translated_stream_baton *b = baton;
+  mark_translated_t *mt = (mark_translated_t *)mark;
+
+  SVN_ERR(svn_stream_remove_mark(b->stream, mt->mark));
+
+  return SVN_NO_ERROR;
+}
+
 /* Implements svn_stream__is_buffered_fn_t. */
 static svn_boolean_t
 translated_stream_is_buffered(void *baton)
@@ -1548,6 +1560,7 @@ stream_translated(svn_stream_t *stream,
   svn_stream_set_close(s, translated_stream_close);
   svn_stream_set_mark(s, translated_stream_mark);
   svn_stream_set_seek(s, translated_stream_seek);
+  svn_stream_set_remove_mark(s, translated_stream_remove_mark);
   svn_stream__set_is_buffered(s, translated_stream_is_buffered);
 
   return s;