stef...@apache.org wrote on Sat, Aug 14, 2010 at 13:20:22 -0000: > Author: stefan2 > Date: Sat Aug 14 13:20:22 2010 > New Revision: 985487 > > URL: http://svn.apache.org/viewvc?rev=985487&view=rev > Log: > APR file I/O is a high frequency operation as the respective streams directly > map their requests > onto the file layer. Thus, introduce svn_io_file_putc() as symmetrical > counterpart to *_getc() > and use both for single char read requests because their APR implementation > tends to involve > far less overhead. > > Also, introduce and use svn_io_file_read_full2 that optionally doesn't create > an error object > upon EOF that may be discarded by the caller anyways. This actually > translates into significant > runtime savings because constructing the svn_error_t for file errors involves > expensive locale > dependent functions. > > * subversion/include/svn_io.h > (svn_io_file_putc, svn_io_file_read_full2): declare new API functions > * subversion/libsvn_subr/svn_io.c > (svn_io_file_putc, svn_io_file_read_full2): implement new API functions > * subversion/libsvn_subr/stream.c > (read_handler_apr, write_handler_apr): use the I/O function best suitable > for the request > > Modified: > subversion/branches/performance/subversion/include/svn_io.h > subversion/branches/performance/subversion/libsvn_subr/io.c > 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=985487&r1=985486&r2=985487&view=diff > ============================================================================== > --- subversion/branches/performance/subversion/include/svn_io.h (original) > +++ subversion/branches/performance/subversion/include/svn_io.h Sat Aug 14 > 13:20:22 2010 > @@ -1745,6 +1745,15 @@ svn_io_file_getc(char *ch, > apr_pool_t *pool); > > > +/** Wrapper for apr_file_putc(). > + * @since New in 1.7 > + */ > +svn_error_t * > +svn_io_file_putc(char ch, > + apr_file_t *file, > + apr_pool_t *pool); > + > + > /** Wrapper for apr_file_info_get(). */ > svn_error_t * > svn_io_file_info_get(apr_finfo_t *finfo, > @@ -1770,6 +1779,20 @@ svn_io_file_read_full(apr_file_t *file, > apr_pool_t *pool); > > > +/** Wrapper for apr_file_read_full(). > + * If eof_is_ok is set, no svn_error_t error object > + * will be created upon EOF. > + * @since New in 1.7 > + */ > +svn_error_t * > +svn_io_file_read_full2(apr_file_t *file, > + void *buf, > + apr_size_t nbytes, > + apr_size_t *bytes_read, > + svn_boolean_t eof_is_ok, > + apr_pool_t *pool); > + > +
Why didn't you deprecate svn_io_file_read_full() in the same commit? Usually we do it as follows: + the declaration of svn_io_file_read_full2() is *above* that of svn_io_file_read_full (not critical) + mark the old function SVN_DEPRECATED (preprocessor symbol) and doxygen @deprecated + change the doc string of the old function to be a diff against the new function's docs + in the appropriate *.c file, upgrade the definition in-place + re-define the old function in lib_$foo/deprecated.c as a wrapper around the new function Why you didn't make svn_io_file_read_full() a deprecated wrapper around svn_io_file_read_full2()? Daniel > /** Wrapper for apr_file_seek(). */ > svn_error_t * > svn_io_file_seek(apr_file_t *file, > > Modified: subversion/branches/performance/subversion/libsvn_subr/io.c > URL: > http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_subr/io.c?rev=985487&r1=985486&r2=985487&view=diff > ============================================================================== > --- subversion/branches/performance/subversion/libsvn_subr/io.c (original) > +++ subversion/branches/performance/subversion/libsvn_subr/io.c Sat Aug 14 > 13:20:22 2010 > @@ -2856,6 +2856,17 @@ svn_io_file_getc(char *ch, apr_file_t *f > > > svn_error_t * > +svn_io_file_putc(char ch, apr_file_t *file, apr_pool_t *pool) > +{ > + return do_io_file_wrapper_cleanup > + (file, apr_file_putc(ch, file), > + N_("Can't write file '%s'"), > + N_("Can't write stream"), > + pool); > +} > + > + > +svn_error_t * > svn_io_file_info_get(apr_finfo_t *finfo, apr_int32_t wanted, > apr_file_t *file, apr_pool_t *pool) > { > @@ -2893,6 +2904,24 @@ svn_io_file_read_full(apr_file_t *file, > > > svn_error_t * > +svn_io_file_read_full2(apr_file_t *file, void *buf, > + apr_size_t nbytes, apr_size_t *bytes_read, > + svn_boolean_t eof_is_ok, > + apr_pool_t *pool) > +{ > + apr_status_t status = apr_file_read_full(file, buf, nbytes, bytes_read); > + if (APR_STATUS_IS_EOF(status) && eof_is_ok) > + return SVN_NO_ERROR; > + > + return do_io_file_wrapper_cleanup > + (file, status, > + N_("Can't read file '%s'"), > + N_("Can't read stream"), > + pool); > +} > + > + > +svn_error_t * > svn_io_file_seek(apr_file_t *file, apr_seek_where_t where, > apr_off_t *offset, apr_pool_t *pool) > { > > Modified: subversion/branches/performance/subversion/libsvn_subr/stream.c > URL: > http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_subr/stream.c?rev=985487&r1=985486&r2=985487&view=diff > ============================================================================== > --- subversion/branches/performance/subversion/libsvn_subr/stream.c (original) > +++ subversion/branches/performance/subversion/libsvn_subr/stream.c Sat Aug > 14 13:20:22 2010 > @@ -600,12 +600,15 @@ read_handler_apr(void *baton, char *buff > struct baton_apr *btn = baton; > svn_error_t *err; > > - err = svn_io_file_read_full(btn->file, buffer, *len, len, btn->pool); > - if (err && APR_STATUS_IS_EOF(err->apr_err)) > + if (*len == 1) > { > - svn_error_clear(err); > - err = SVN_NO_ERROR; > - } > + err = svn_io_file_getc (buffer, btn->file, btn->pool); > + if (err) > + *len = 0; > + } > + else > + err = svn_io_file_read_full2(btn->file, buffer, *len, len, > + TRUE, btn->pool); > > return err; > } > @@ -614,8 +617,18 @@ static svn_error_t * > write_handler_apr(void *baton, const char *data, apr_size_t *len) > { > struct baton_apr *btn = baton; > + svn_error_t *err; > + > + if (*len == 1) > + { > + err = svn_io_file_putc (*data, btn->file, btn->pool); > + if (err) > + *len = 0; > + } > + else > + err = svn_io_file_write_full(btn->file, data, *len, len, btn->pool); > > - return svn_io_file_write_full(btn->file, data, *len, len, btn->pool); > + return err; > } > > static svn_error_t * > >
