Return-Path: 
 <commits-return-16611-apmail-subversion-commits-archive=subversion.apache.org@subversion.apache.org>
X-Original-To: apmail-subversion-commits-archive@minotaur.apache.org
Delivered-To: apmail-subversion-commits-archive@minotaur.apache.org
Received: from mail.apache.org (hermes.apache.org [140.211.11.3])
	by minotaur.apache.org (Postfix) with SMTP id 35E9175FE
	for <apmail-subversion-commits-archive@minotaur.apache.org>;
 Wed, 26 Oct 2011 12:42:38 +0000 (UTC)
Received: (qmail 8729 invoked by uid 500); 26 Oct 2011 12:42:37 -0000
Delivered-To: apmail-subversion-commits-archive@subversion.apache.org
Received: (qmail 8694 invoked by uid 500); 26 Oct 2011 12:42:37 -0000
Mailing-List: contact commits-help@subversion.apache.org; run by ezmlm
Precedence: bulk
List-Help: <mailto:commits-help@subversion.apache.org>
List-Unsubscribe: <mailto:commits-unsubscribe@subversion.apache.org>
List-Post: <mailto:commits@subversion.apache.org>
List-Id: <commits.subversion.apache.org>
Reply-To: dev@subversion.apache.org
Delivered-To: mailing list commits@subversion.apache.org
Received: (qmail 8680 invoked by uid 99); 26 Oct 2011 12:42:37 -0000
Received: from athena.apache.org (HELO athena.apache.org) (140.211.11.136)
    by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 26 Oct 2011 12:42:37 +0000
X-ASF-Spam-Status: No, hits=2.2 required=5.0
	tests=HTML_MESSAGE,RCVD_IN_DNSWL_LOW,SPF_NEUTRAL
X-Spam-Check-By: apache.org
Received-SPF: neutral (athena.apache.org: local policy)
Received: from [74.125.82.47] (HELO mail-ww0-f47.google.com) (74.125.82.47)
    by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 26 Oct 2011 12:42:32 +0000
Received: by wwg7 with SMTP id 7so1664227wwg.16
        for <multiple recipients>; Wed, 26 Oct 2011 05:42:10 -0700 (PDT)
MIME-Version: 1.0
Received: by 10.216.156.204 with SMTP id m54mr12156149wek.27.1319632930679;
 Wed, 26 Oct 2011 05:42:10 -0700 (PDT)
Received: by 10.216.85.69 with HTTP; Wed, 26 Oct 2011 05:42:09 -0700 (PDT)
In-Reply-To: <20111026100701.79AC2238889B@eris.apache.org>
References: <20111026100701.79AC2238889B@eris.apache.org>
Date: Wed, 26 Oct 2011 07:42:09 -0500
Message-ID: 
 <CAJjMeYM0-PuQmx+_21ucVoUVFZSAAb4r96QTdgWpfC3SU7HYrw@mail.gmail.com>
Subject: Re: svn commit: r1189103 -
 /subversion/trunk/subversion/libsvn_subr/mergeinfo.c
From: Hyrum K Wright <hyrum.wright@wandisco.com>
To: dev@subversion.apache.org
Cc: commits@subversion.apache.org
Content-Type: multipart/alternative; boundary=0016e65b4b8a112bef04b032fcc8

--0016e65b4b8a112bef04b032fcc8
Content-Type: text/plain; charset=UTF-8

On Wed, Oct 26, 2011 at 5:07 AM, <julianfoad@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/

--0016e65b4b8a112bef04b032fcc8
Content-Type: text/html; charset=UTF-8
Content-Transfer-Encoding: quoted-printable

<br><br><div class=3D"gmail_quote">On Wed, Oct 26, 2011 at 5:07 AM,  <span =
dir=3D"ltr">&lt;<a href=3D"mailto:julianfoad@apache.org">julianfoad@apache.=
org</a>&gt;</span> wrote:<br><blockquote class=3D"gmail_quote" style=3D"mar=
gin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex;">
Author: julianfoad<br>
Date: Wed Oct 26 10:07:01 2011<br>
New Revision: 1189103<br>
<br>
URL: <a href=3D"http://svn.apache.org/viewvc?rev=3D1189103&amp;view=3Drev" =
target=3D"_blank">http://svn.apache.org/viewvc?rev=3D1189103&amp;view=3Drev=
</a><br>
Log:<br>
* subversion/libsvn_subr/mergeinfo.c<br>
 =C2=A0(svn_mergeinfo_to_string): Simplify by eliminating special-casing of=
 N=3D0.<br></blockquote><div><br></div><div>While I appreciate the need to =
have concise code, it wouldn&#39;t surprise me if the special case was in t=
here for performance reasons. =C2=A0If the no-mergeinfo case is very common=
, avoiding superfluous calls to mergeinfo_to_string() and=C2=A0svn_stringbu=
f__morph_into_string() could have quite an impact.=C2=A0 I haven&#39;t look=
ed at the logs, but it feels like just the sort of thing that Stefan2 would=
 do. :)</div>
<div><br></div><div>-Hyrum</div><div>=C2=A0</div><blockquote class=3D"gmail=
_quote" style=3D"margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:=
1ex;">
<br>
Modified:<br>
 =C2=A0 =C2=A0subversion/trunk/subversion/libsvn_subr/mergeinfo.c<br>
<br>
Modified: subversion/trunk/subversion/libsvn_subr/mergeinfo.c<br>
URL: <a href=3D"http://svn.apache.org/viewvc/subversion/trunk/subversion/li=
bsvn_subr/mergeinfo.c?rev=3D1189103&amp;r1=3D1189102&amp;r2=3D1189103&amp;v=
iew=3Ddiff" target=3D"_blank">http://svn.apache.org/viewvc/subversion/trunk=
/subversion/libsvn_subr/mergeinfo.c?rev=3D1189103&amp;r1=3D1189102&amp;r2=
=3D1189103&amp;view=3Ddiff</a><br>

=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D<br>
--- subversion/trunk/subversion/libsvn_subr/mergeinfo.c (original)<br>
+++ subversion/trunk/subversion/libsvn_subr/mergeinfo.c Wed Oct 26 10:07:01=
 2011<br>
@@ -1923,16 +1923,10 @@ svn_error_t *<br>
=C2=A0svn_mergeinfo_to_string(svn_string_t **output, svn_mergeinfo_t input,=
<br>
 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=
=A0 =C2=A0 apr_pool_t *pool)<br>
=C2=A0{<br>
- =C2=A0if (apr_hash_count(input) &gt; 0)<br>
- =C2=A0 =C2=A0{<br>
- =C2=A0 =C2=A0 =C2=A0svn_stringbuf_t *mergeinfo_buf;<br>
- =C2=A0 =C2=A0 =C2=A0SVN_ERR(mergeinfo_to_stringbuf(&amp;mergeinfo_buf, in=
put, NULL, pool));<br>
- =C2=A0 =C2=A0 =C2=A0*output =3D svn_stringbuf__morph_into_string(mergeinf=
o_buf);<br>
- =C2=A0 =C2=A0}<br>
- =C2=A0else<br>
- =C2=A0 =C2=A0{<br>
- =C2=A0 =C2=A0 =C2=A0*output =3D svn_string_create(&quot;&quot;, pool);<br=
>
- =C2=A0 =C2=A0}<br>
+ =C2=A0svn_stringbuf_t *mergeinfo_buf;<br>
+<br>
+ =C2=A0SVN_ERR(mergeinfo_to_stringbuf(&amp;mergeinfo_buf, input, NULL, poo=
l));<br>
+ =C2=A0*output =3D svn_stringbuf__morph_into_string(mergeinfo_buf);<br>
 =C2=A0 return SVN_NO_ERROR;<br>
=C2=A0}<br>
<br>
<br>
<br>
</blockquote></div><br><br clear=3D"all"><div><br></div>-- <br><br>uberSVN:=
 Apache Subversion Made Easy<br><a href=3D"http://www.uberSVN.com/" target=
=3D"_blank">http://www.uberSVN.com/</a><br>

--0016e65b4b8a112bef04b032fcc8--