> -----Original Message-----
> From: Stefan Fuhrmann [mailto:stefanfuhrmann_at_alice-dsl.de]
> Sent: zaterdag 1 mei 2010 19:03
> To: dev_at_subversion.apache.org
> Subject: [PATCH v2] Saving a few cycles, part 3/3
>
> Hi devs,
>
> I reworked the patch set according to the feedback I got
> here on this list. This is what changed in this last part:
>
> * add rationales to the commentary
> * bring svndiff.c changes closer to the original code
> * use svn_ctype_* functions in checksum parser
>
> -- Stefan^2.
>
> [[[
> Various local optimizations. These opportunities became
> visible after significantly optimizing other parts of svn. The
> total savings for a 'svn export' is almost 3 percent on top
> of the previous.
>
> The largest savings can be attributed to the svndiff.c
> changes (~1.5%) and the checksum parser (~1%).
>
> * subversion/include/svn_delta.h
> (enum svn_delta_action): ensure that these definitions
> will always have these values, even if new definitions
> were added
This part is not necessary. These values are part of our public API
compatibility promise and that makes the values static for 1.X.
The C compiler defines the same values... and if it didn't you are not
allowed to change the values (for the same binary compatibility reasons).
In the rest of your patch you can use the enum constants in some places
instead of the magic values to increase some readability. (But I think the
old code didn't do that either).
>
> * subversion/libsvn_delta/compose_delta.c
> (search_offset_index): use size_t to index memory.
> This is mainly a consistency fix but may actually result
> in slightly higher performance due to fewer conversions.
s/size_t/apr_size_t/
And it can also cause a slowdown. But I like to see apr_size_t anyway.
> (copy_source_ops): dito; optimize a common case
>
> * subversion/libsvn_delta/svndiff.c
> (decode_file_offset, decode_size): use slightly more
> efficient formulations
> (decode_instruction): directly map action codes -
> made possible by defining the enum values
The enum values were already defined so that doesn't change anything (see
above: they can't change anyway).
Optimizing a % operator for values of 1 and 2 looks like code obfuscation to
me, unless you can show me some numbers that it actually makes a difference.
I would assume that the processor optimizes that case itself.
>
> * subversion/libsvn_subr/checksum.c
> (svn_checksum_parse_hex): eliminate calls to locale-
> aware CRT functions. At least with MS compilers,
> these are very expensive.
Yes, these call into the OS-libraries on Windows.. Avoiding these helps *a
lot*.
> * subversion/libsvn_subr/stream.c
> (stream_readline): numbytes is invariant until EOF
>
> patch by stefanfuhrmann < at > alice-dsl.de
> ]]]
Bert
Received on 2010-05-01 21:37:19 CEST