[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r982375 - in /subversion/branches/performance/subversion: include/svn_io.h libsvn_subr/stream.c

From: Hyrum K. Wright <hyrum_wright_at_mail.utexas.edu>
Date: Wed, 4 Aug 2010 14:34:19 -0500

On Wed, Aug 4, 2010 at 2:27 PM, <stefan2_at_apache.org> wrote:
> Author: stefan2
> Date: Wed Aug  4 19:27:41 2010
> New Revision: 982375
>
> URL: http://svn.apache.org/viewvc?rev=982375&view=rev
> Log:
> Introduce a variant of svn_stream_from_aprfile2, that takes cached file handles
> instead of APR file handles.
>
> * subversion/include/svn_io.h
>  (svn_stream_from_aprfile3): declare new API function

Is this intended as a replacement for svn_stream_from_aprfile2()? If
so, the the later should be deprecated. If not, I would suggest a new
name, since we generally only have one non-deprecated version of
svn_fooN() APIs in any given release. It's okay for the separate
functions to exist, but they shouldn't be related via the naming
ancestry.

-Hyrum

> * subversion/libsvn_subr/stream.c
>  (baton_apr): add cached_handle member
>  (close_handler_cached_handle): new close() function
>  (stream_from_aprfile): extract generic part from svn_stream_from_aprfile2
>  (svn_stream_from_aprfile2): call stream_from_aprfile for most of the init code
>  (svn_stream_from_aprfile3): implement new API function
>
> Modified:
>    subversion/branches/performance/subversion/include/svn_io.h
>    subversion/branches/performance/subversion/libsvn_subr/stream.c
>
> Modified: subversion/branches/performance/subversion/include/svn_io.h
> URL: http://svn.apache.org/viewvc/subversion/branches/performance/subversion/include/svn_io.h?rev=982375&r1=982374&r2=982375&view=diff
> ==============================================================================
> --- subversion/branches/performance/subversion/include/svn_io.h (original)
> +++ subversion/branches/performance/subversion/include/svn_io.h Wed Aug  4 19:27:41 2010
> @@ -923,6 +923,26 @@ svn_stream_from_aprfile2(apr_file_t *fil
>                          svn_boolean_t disown,
>                          apr_pool_t *pool);
>
> +/* "forward-declare" svn_file_handle_cache__handle_t */
> +struct svn_file_handle_cache__handle_t;
> +
> +/** Create a stream from a cached file handle.  For convenience, if @a file
> + * is @c NULL, an empty stream created by svn_stream_empty() is returned.
> + *
> + * This function should normally be called with @a disown set to FALSE,
> + * in which case closing the stream will also return the file handle to
> + * the respective cache object.
> + *
> + * If @a disown is TRUE, the stream will disown the file handle, meaning
> + * that svn_stream_close() will not close the cached file handle.
> + *
> + * @since New in 1.7.
> + */
> +svn_stream_t *
> +svn_stream_from_aprfile3(struct svn_file_handle_cache__handle_t *file,
> +                         svn_boolean_t disown,
> +                         apr_pool_t *pool);
> +
>  /** Similar to svn_stream_from_aprfile2(), except that the file will
>  * always be disowned.
>  *
>
> Modified: subversion/branches/performance/subversion/libsvn_subr/stream.c
> URL: http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_subr/stream.c?rev=982375&r1=982374&r2=982375&view=diff
> ==============================================================================
> --- subversion/branches/performance/subversion/libsvn_subr/stream.c (original)
> +++ subversion/branches/performance/subversion/libsvn_subr/stream.c Wed Aug  4 19:27:41 2010
> @@ -43,7 +43,7 @@
>  #include "svn_checksum.h"
>  #include "svn_path.h"
>  #include "private/svn_eol_private.h"
> -
> +#include "private/svn_file_handle_cache.h"
>
>  struct svn_stream_t {
>   void *baton;
> @@ -579,6 +579,10 @@ struct baton_apr {
>   apr_file_t *file;
>   apr_pool_t *pool;
>
> +  /* If not NULL, file is actually wrapped into this cached file handle
> +   * and should be returned to the file handle cache asap. */
> +  svn_file_handle_cache__handle_t *cached_handle;
> +
>   /* Offsets when reading from a range of the file.
>    * When either of these is negative, no range has been specified. */
>   apr_off_t start;
> @@ -622,6 +626,17 @@ close_handler_apr(void *baton)
>   return svn_io_file_close(btn->file, btn->pool);
>  }
>
> +/* Returns the cached file handle back to the respective cache object.
> + * This is call is allowed even for btn->cached_handle == NULL.
> + */
> +static svn_error_t *
> +close_handler_cached_handle(void *baton)
> +{
> +  struct baton_apr *btn = baton;
> +
> +  return svn_file_handle_cache__close(btn->cached_handle);
> +}
> +
>  static svn_error_t *
>  reset_handler_apr(void *baton)
>  {
> @@ -713,36 +728,83 @@ svn_stream_open_unique(svn_stream_t **st
>   return SVN_NO_ERROR;
>  }
>
> +/* Common initialization code for svn_stream_from_aprfile2() and
> + * svn_stream_from_aprfile3().
> + */
> +static svn_stream_t *
> +stream_from_aprfile(struct baton_apr **baton,
> +                    apr_file_t *file,
> +                    apr_pool_t *pool)
> +{
> +  struct baton_apr *new_baton;
> +  svn_stream_t *stream;
> +
> +  /* create and fully initialize the baton */
> +  new_baton = apr_palloc(pool, sizeof(*new_baton));
> +  new_baton->file = file;
> +  new_baton->cached_handle = NULL; /* default */
> +  new_baton->pool = pool;
> +  new_baton->start = -1;
> +  new_baton->end = -1;
> +
> +  /* construct the stream vtable, except for the close() function */
> +  stream = svn_stream_create(new_baton, pool);
> +  svn_stream_set_read(stream, read_handler_apr);
> +  svn_stream_set_write(stream, write_handler_apr);
> +  svn_stream_set_reset(stream, reset_handler_apr);
> +  svn_stream_set_mark(stream, mark_handler_apr);
> +  svn_stream_set_seek(stream, seek_handler_apr);
> +
> +  /* return structures */
> +  *baton = new_baton;
> +
> +  return stream;
> +}
>
>  svn_stream_t *
>  svn_stream_from_aprfile2(apr_file_t *file,
>                          svn_boolean_t disown,
>                          apr_pool_t *pool)
>  {
> +  /* having no file at all is a special case */
>   struct baton_apr *baton;
> -  svn_stream_t *stream;
> -
>   if (file == NULL)
>     return svn_stream_empty(pool);
>
> -  baton = apr_palloc(pool, sizeof(*baton));
> -  baton->file = file;
> -  baton->pool = pool;
> -  baton->start = -1;
> -  baton->end = -1;
> -  stream = svn_stream_create(baton, pool);
> -  svn_stream_set_read(stream, read_handler_apr);
> -  svn_stream_set_write(stream, write_handler_apr);
> -  svn_stream_set_reset(stream, reset_handler_apr);
> -  svn_stream_set_mark(stream, mark_handler_apr);
> -  svn_stream_set_seek(stream, seek_handler_apr);
> +  /* construct and init the default stream structures */
> +  svn_stream_t *stream = stream_from_aprfile(&baton, file, pool);
>
> +  /* make sure to close the file handle after use if we own it */
>   if (! disown)
>     svn_stream_set_close(stream, close_handler_apr);
>
>   return stream;
>  }
>
> +svn_stream_t *
> +svn_stream_from_aprfile3(svn_file_handle_cache__handle_t *file,
> +                         svn_boolean_t disown,
> +                         apr_pool_t *pool)
> +{
> +  struct baton_apr *baton;
> +
> +  /* having no file at all is a special case (file == NULL is legal, too) */
> +  apr_file_t *apr_file = svn_file_handle_cache__get_apr_handle(file);
> +  if (apr_file == NULL)
> +    return svn_stream_empty(pool);
> +
> +  /* construct and init the default stream structures */
> +  svn_stream_t *stream = stream_from_aprfile(&baton, apr_file, pool);
> +
> +  /* store the cached file handle and return it to the cache after use,
> +   * if we own it */
> +  baton->cached_handle = file;
> +  if (! disown)
> +    svn_stream_set_close(stream, close_handler_cached_handle);
> +
> +  return stream;
> +}
> +
>  /* A read handler (#svn_read_fn_t) that forwards to read_handler_apr()
>    but only allows reading between BATON->start and BATON->end. */
>  static svn_error_t *
>
>
>
Received on 2010-08-04 21:35:02 CEST

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.