Daniel Shahaf wrote:
> stefan2_at_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?
>
By the time I wrote that code almost 3 months ago, I tried to minimize
the impact (code churn) on the code base.
> 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
>
Took me a number of commits but it should be o.k. by r986492.
> Why you didn't make svn_io_file_read_full() a deprecated wrapper around
> svn_io_file_read_full2()?
>
See above ;)
-- Stefan^2.
Received on 2010-08-17 23:43:58 CEST