On Wed, Aug 4, 2010 at 2:27 PM, <stef...@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 * > > >