subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Branko ─îibej <br...@wandisco.com>
Subject Re: svn commit: r1483292 - in /subversion/trunk/subversion: include/private/svn_string_private.h libsvn_subr/string.c libsvn_subr/types.c
Date Fri, 17 May 2013 05:53:14 GMT
On 16.05.2013 12:19, stefan2@apache.org wrote:
> Author: stefan2
> Date: Thu May 16 10:19:18 2013
> New Revision: 1483292
>
> URL: http://svn.apache.org/r1483292
> Log:
> Daring another no-no: provide our own, optimized implementation
> of a string to unsigned int conversion.  Use it to simplify & speed up
> svn_revnum_parse.
>
> Background: In my 'svn log -g' tests, I found that >50% of the runtime
> is spent parsing mergeinfo and ~20% in strtol alone.
>
> * subversion/include/private/svn_string_private.h
>   (svn__strtoff): declare new private API
>
> * subversion/libsvn_subr/string.c
>   (svn__strtoff): implement it
>
> * subversion/libsvn_subr/types.c
>   (svn_revnum_parse): use it
>
> Modified:
>     subversion/trunk/subversion/include/private/svn_string_private.h
>     subversion/trunk/subversion/libsvn_subr/string.c
>     subversion/trunk/subversion/libsvn_subr/types.c
>
> Modified: subversion/trunk/subversion/include/private/svn_string_private.h
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_string_private.h?rev=1483292&r1=1483291&r2=1483292&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/private/svn_string_private.h (original)
> +++ subversion/trunk/subversion/include/private/svn_string_private.h Thu May 16 10:19:18
2013
> @@ -136,6 +136,12 @@ svn_stringbuf__morph_into_string(svn_str
>  apr_status_t
>  svn__strtoff(apr_off_t *offset, const char *buf, char **end, int base);
>  
> +/** Like strtoul but with a fixed base of 10.  This allows the compiler to
> + * generate massively faster (4x on 64bit LINUX) code.
> + */
> +unsigned long
> +svn__strtoul(const char *buffer, char **end);
> +
>  /** Number of chars needed to represent signed (19 places + sign + NUL) or
>   * unsigned (20 places + NUL) integers as strings.
>   */
>
> Modified: subversion/trunk/subversion/libsvn_subr/string.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/string.c?rev=1483292&r1=1483291&r2=1483292&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/string.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/string.c Thu May 16 10:19:18 2013
> @@ -1027,6 +1027,36 @@ svn__strtoff(apr_off_t *offset, const ch
>  #endif
>  }
>  
> +unsigned long
> +svn__strtoul(const char *buffer, char **end)
> +{
> +  unsigned long result = 0;
> +
> +  /* this loop will execute in just 2 CPU cycles, confirmed by measurement:
> +     7 macro-ops (max 4 / cycle => 2 cycles)

You know, I really don't find these kinds of comments very helpful
unless they also say that they're specific to one particular version of
a particular CPU architecture ...

> +       1 load (max 1 / cycle)
> +       1 jumps (compare + conditional jump == 1 macro op; max 1 / cycle)
> +       2 arithmetic ops (subtract, increment; max 3 / cycle)
> +       2 scale-and-add AGU ops (max 3 / cycle)
> +       1 compiler-generated move operation
> +     dependency chain: temp = result * 4 + result; result = temp * 2 + c
> +                       (2 ops with latency 1 => 2 cycles)
> +   */
> +  while (1)
> +    {
> +      unsigned long c = *buffer - '0';
> +      if (c > 9)
> +        break;
> +
> +      result = result * 10 + c;
> +      ++buffer;
> +    }
> +
> +  *end = (char *)buffer;

And this is why I prefer not to reimplement library functions. Why is
'end' a 'char**' and not a 'char *const *'? I always felt the strtol
signature was massively broken because of that.

-- Brane


-- 
Branko ─îibej
Director of Subversion | WANdisco | www.wandisco.com


Mime
View raw message