On Sun, 2010-03-07 at 20:40 +0100, Stefan Fuhrman wrote:
> While profiling TSVN's SVN proper access code,
> I found that about 40% of the runtime was spent in
> svn_config_get_config(). The reason was the config
> parser fetching individual bytes from the input stream.
>
> However, translated_stream_read() obviously imposes
> a significant overhead per call for smaller chunk sizes.
> This patch quadruples the translated_stream_read()
> throughput for single bytes cutting svn_config_get_config()
> runtime by 50%.
This is a good example of optimisation: with profiling evidence, and
with the optimised code path being simple, and well documented.
+1 to this.
There's a small mistake in the implementation: ...
> [[[
> Speed up input stream processing in config parser and
> others that read single bytes from a stream.
>
> * subversion/libsvn_subr/subst.c
> (translated_stream_read): Add an optimized code path
> for single byte read requests.
>
> Patch by: Stefan Fuhrmann <stefanfuhrmann{_AT_}alice-dsl.de>
> ]]]
>
> Index: subversion/libsvn_subr/subst.c
> ===================================================================
> --- subversion/libsvn_subr/subst.c (revision 920055)
> +++ subversion/libsvn_subr/subst.c (working copy)
> @@ -1044,6 +1044,24 @@
> apr_size_t off = 0;
> apr_pool_t *iterpool;
>
> + /* Optimization for a frequent special case:
> + The configuration parser (and a few others) reads the
> + stream one byte at a time. All the memcpy, pool clearing
> + etc. imposes a huge overhead in that case. In most cases,
> + we can just take that single byte directly from the read
> + buffer.
> + Since *len > 1 requires lots of code to be run anyways,
> + we can afford the extra overhead of checking for *len == 1. */
> + if (unsatisfied == 1 && b->readbuf_off < b->readbuf->len)
> + {
> + /* Just take it from the read buffer */
> + *buffer = b->readbuf->data[b->readbuf_off++];
> + *len = 0;
"*len" should be set to the amount that actually was read - in other
words unchanged from its supplied value of 1 - so that last assignment
should be deleted.
Other than that, it looks perfect.
I'll commit if (when) all tests pass. (I'll mention the profiling in
the log message or in the code.)
- Julian
> + return SVN_NO_ERROR;
> + }
> +
> + /* Standard code path. */
> iterpool = b->iterpool;
> while (readlen == SVN__STREAM_CHUNK_SIZE && unsatisfied > 0)
> {
Received on 2010-03-09 19:32:14 CET