You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Hyrum K Wright <hy...@wandisco.com> on 2011/10/26 14:42:09 UTC

Re: svn commit: r1189103 - /subversion/trunk/subversion/libsvn_subr/mergeinfo.c

On Wed, Oct 26, 2011 at 5:07 AM, <ju...@apache.org> wrote:

> Author: julianfoad
> Date: Wed Oct 26 10:07:01 2011
> New Revision: 1189103
>
> URL: http://svn.apache.org/viewvc?rev=1189103&view=rev
> Log:
> * subversion/libsvn_subr/mergeinfo.c
>  (svn_mergeinfo_to_string): Simplify by eliminating special-casing of N=0.
>

While I appreciate the need to have concise code, it wouldn't surprise me if
the special case was in there for performance reasons.  If the no-mergeinfo
case is very common, avoiding superfluous calls to mergeinfo_to_string()
and svn_stringbuf__morph_into_string() could have quite an impact.  I
haven't looked at the logs, but it feels like just the sort of thing that
Stefan2 would do. :)

-Hyrum


>
> Modified:
>    subversion/trunk/subversion/libsvn_subr/mergeinfo.c
>
> Modified: subversion/trunk/subversion/libsvn_subr/mergeinfo.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/mergeinfo.c?rev=1189103&r1=1189102&r2=1189103&view=diff
>
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/mergeinfo.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/mergeinfo.c Wed Oct 26 10:07:01
> 2011
> @@ -1923,16 +1923,10 @@ svn_error_t *
>  svn_mergeinfo_to_string(svn_string_t **output, svn_mergeinfo_t input,
>                         apr_pool_t *pool)
>  {
> -  if (apr_hash_count(input) > 0)
> -    {
> -      svn_stringbuf_t *mergeinfo_buf;
> -      SVN_ERR(mergeinfo_to_stringbuf(&mergeinfo_buf, input, NULL, pool));
> -      *output = svn_stringbuf__morph_into_string(mergeinfo_buf);
> -    }
> -  else
> -    {
> -      *output = svn_string_create("", pool);
> -    }
> +  svn_stringbuf_t *mergeinfo_buf;
> +
> +  SVN_ERR(mergeinfo_to_stringbuf(&mergeinfo_buf, input, NULL, pool));
> +  *output = svn_stringbuf__morph_into_string(mergeinfo_buf);
>   return SVN_NO_ERROR;
>  }
>
>
>
>


-- 

uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com/

Re: svn commit: r1189103 - /subversion/trunk/subversion/libsvn_subr/mergeinfo.c

Posted by Philip Martin <ph...@wandisco.com>.
Hyrum K Wright <hy...@wandisco.com> writes:

> While I appreciate the need to have concise code, it wouldn't surprise me if
> the special case was in there for performance reasons.  If the no-mergeinfo
> case is very common, avoiding superfluous calls to mergeinfo_to_string()
> and svn_stringbuf__morph_into_string() could have quite an impact.

mergeinfo_to_stringbuf has the same optimisation.

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com

Re: svn commit: r1189103 - /subversion/trunk/subversion/libsvn_subr/mergeinfo.c

Posted by Philip Martin <ph...@wandisco.com>.
Hyrum K Wright <hy...@wandisco.com> writes:

> While I appreciate the need to have concise code, it wouldn't surprise me if
> the special case was in there for performance reasons.  If the no-mergeinfo
> case is very common, avoiding superfluous calls to mergeinfo_to_string()
> and svn_stringbuf__morph_into_string() could have quite an impact.

mergeinfo_to_stringbuf has the same optimisation.

-- 
uberSVN: Apache Subversion Made Easy
http://www.uberSVN.com