[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: svn commit: r1483532 - in /subversion/trunk/subversion:

From: Bert Huijben <bert_at_qqmail.nl>
Date: Fri, 17 May 2013 05:15:41 -0700

 include/ libsvn_client/ libsvn_fs_base/ libsvn_fs_fs/ libsvn_ra/

 libsvn_ra_local/ libsvn_ra_svn/ libsvn_repos/ libsvn_subr/ libsvn_wc/

 mod_dav_svn/ svndumpfilter/ svnmucc/ svnrdump/ svnserve/ svn
MIME-Version: 1.0
Content-Type: multipart/alternative; boundary=001a11c1f7a4da3bca04dce7228d

--001a11c1f7a4da3bca04dce7228d
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Maybe we should update the _gets and _puts then, as most likely the
only thing the constant does is delaying that call.

This has the nice side effect of adding a type check to these macros.

Note that in general the number of size checks should be very low for
the hash table. If it isn't that is a pretty good proof that the hash
function isn't good for the specific data set.

Bert From: Stefan Fuhrmann
Sent: =E2=80=8E17/=E2=80=8E05/=E2=80=8E2013 12:06
To: Julian Foad
Cc: Bert Huijben; Ivan Zhakov; stefan2_at_apache.org;
dev_at_subversion.apache.org
Subject: Re: svn commit: r1483532 - in /subversion/trunk/subversion:
include/ libsvn_client/ libsvn_fs_base/ libsvn_fs_fs/ libsvn_ra/
libsvn_ra_local/ libsvn_ra_svn/ libsvn_repos/ libsvn_subr/ libsvn_wc/
mod_dav_svn/ svndumpfilter/ svnmucc/ svnrdump/ svnserve/ svn
On Fri, May 17, 2013 at 12:23 AM, Julian Foad <julianfoad_at_btopenworld.com>w=
rote:

> Bert Huijben wrote:
> > Ivan Zhakov wrote:
> >>> Author: stefan2
> >>> URL: http://svn.apache.org/r1483532
> >>> Log:
> >>> We frequently use property name constants in conjunction with hash
> >>> containers. Provide new wrappers around apr_hash_get and apr_hash_set
> >>> that accept such string constants and statically determine their size=
.
> >>> That minimizes the hash access costs.
> >>>
> >>> Mass change hash get and set calls for SVN_PROP_* constants.
> >>
> >> Is the performance gain costs code complexity? Please understand my
> >> correctly: it's great improve Subversion speed. I just don't like
> >> the idea getting code more complicated to win just several cycles.
> >
> > I can see this change making some difference when used in a tight loop
> in fsfs,
> > but in almost every case updated here it is a single call per 'svn' (or
> > other tool) invocation and I don't think the added complexity and the
> risk
> > of accidentally applying it to a pointer in the future makes sense here=
.
> The
> > cost of a strlen on a string of a few characters certainly isn't that
> high.
> >
> > The IO overhead is so much larger that global changes like this in the
> higher
> > layers don't make any sense to me.
>
> A safer and simpler design to achieve the same goal as this could be:
>
> #define svn_hash_gets(ht, key) \
> svn__internal_hash_get(ht, key, strlen(key))
>
> which works because the compiler can then optimize out the "strlen" when
> it is safe and possible to do so.
>

Excellent idea. I just tried it and at least GCC is clever enough
to do that optimization. I'll commit that change tonight.

-- Stefan^2.

--=20
*Join one of our free daily demo sessions on* *Scaling Subversion for the
Enterprise <http://www.wandisco.com/training/webinars>*
*

*

--001a11c1f7a4da3bca04dce7228d
Content-Type: text/html; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

<HTML><HEAD>
<META content=3D"text/html; charset=3Dutf-8" http-equiv=3DContent-Type></HE=
AD>
<BODY>
<DIV>
<DIV style=3D"FONT-SIZE: 11pt; FONT-FAMILY: Calibri,sans-serif">Maybe we sh=
ould update the _gets and _puts then, as most likely the only thing the con=
stant does is delaying that call.<BR><BR>This has the nice side effect of a=
dding a type check to these macros.<BR><BR>Note that in general the number =
of size checks should be very low for the hash table. If it isn't that is a=
 pretty good proof that the hash function isn't good for the specific data =
set.<BR><BR>Bert</DIV></DIV>
<DIV dir=3Dltr>
<HR>
<SPAN style=3D"FONT-SIZE: 11pt; FONT-FAMILY: Calibri,sans-serif; FONT-WEIGH=
T: bold">From: </SPAN><SPAN style=3D"FONT-SIZE: 11pt; FONT-FAMILY: Calibri,=
sans-serif"><A href=3D"mailto:stefan.fuhrmann_at_wandisco.com">Stefan Fuhrmann=
</A></SPAN><BR><SPAN style=3D"FONT-SIZE: 11pt; FONT-FAMILY: Calibri,sans-se=
rif; FONT-WEIGHT: bold">Sent: </SPAN><SPAN style=3D"FONT-SIZE: 11pt; FONT-F=
AMILY: Calibri,sans-serif">=E2=80=8E17/=E2=80=8E05/=E2=80=8E2013 12:06</SPA=
N><BR><SPAN style=3D"FONT-SIZE: 11pt; FONT-FAMILY: Calibri,sans-serif; FONT=
-WEIGHT: bold">To: </SPAN><SPAN style=3D"FONT-SIZE: 11pt; FONT-FAMILY: Cali=
bri,sans-serif"><A href=3D"mailto:julianfoad_at_btopenworld.com">Julian Foad</=
A></SPAN><BR><SPAN style=3D"FONT-SIZE: 11pt; FONT-FAMILY: Calibri,sans-seri=
f; FONT-WEIGHT: bold">Cc: </SPAN><SPAN style=3D"FONT-SIZE: 11pt; FONT-FAMIL=
Y: Calibri,sans-serif"><A href=3D"mailto:bert_at_qqmail.nl">Bert Huijben</A>; =
<A href=3D"mailto:ivan_at_visualsvn.com">Ivan Zhakov</A>; <A href=3D"mailto:st=
efan2_at_apache.org">stefan2_at_apache.org</A>; <A href=3D"mailto:dev_at_subversion.=
apache.org">dev_at_subversion.apache.org</A></SPAN><BR><SPAN style=3D"FONT-SIZ=
E: 11pt; FONT-FAMILY: Calibri,sans-serif; FONT-WEIGHT: bold">Subject: </SPA=
N><SPAN style=3D"FONT-SIZE: 11pt; FONT-FAMILY: Calibri,sans-serif">Re: svn =
commit: r1483532 - in /subversion/trunk/subversion: include/ libsvn_client/=
 libsvn_fs_base/ libsvn_fs_fs/ libsvn_ra/ libsvn_ra_local/ libsvn_ra_svn/ l=
ibsvn_repos/ libsvn_subr/ libsvn_wc/ mod_dav_svn/ svndumpfilter/ svnmucc/ s=
vnrdump/ svnserve/ svn</SPAN><BR><BR></DIV></BODY></HTML><div dir=3D"ltr"><=
div class=3D"gmail_extra"><div class=3D"gmail_quote">On Fri, May 17, 2013 a=
t 12:23 AM, Julian Foad <span dir=3D"ltr">&lt;<a href=3D"mailto:julianfoad@=
btopenworld.com" target=3D"_blank">julianfoad_at_btopenworld.com</a>&gt;</span=
> wrote:<br>
<blockquote class=3D"gmail_quote" style=3D"margin:0 0 0 .8ex;border-left:1p=
x #ccc solid;padding-left:1ex">Bert Huijben wrote:<br>
&gt; Ivan Zhakov wrote:<br>
&gt;&gt;&gt; Author: stefan2<br>
<div class=3D"im">&gt;&gt;&gt; URL: <a href=3D"http://svn.apache.org/r14835=
32" target=3D"_blank">http://svn.apache.org/r1483532</a><br>
&gt;&gt;&gt; Log:<br>
&gt;&gt;&gt; We frequently use property name constants in conjunction with =
hash<br>
&gt;&gt;&gt; containers. Provide new wrappers around apr_hash_get and apr_h=
ash_set<br>
&gt;&gt;&gt; that accept such string constants and statically determine the=
ir size.<br>
&gt;&gt;&gt; That minimizes the hash access costs.<br>
&gt;&gt;&gt;<br>
&gt;&gt;&gt; Mass change hash get and set calls for SVN_PROP_* constants.<b=
r>
&gt;&gt;<br>
</div><div class=3D"im">&gt;&gt; Is the performance gain costs code complex=
ity? Please understand my<br>
&gt;&gt; correctly: it&#39;s great improve Subversion speed. I just don&#39=
;t like<br>
&gt;&gt; the idea getting code more complicated to win just several cycles.=
<br>
&gt;<br>
&gt; I can see this change making some difference when used in a tight loop=
 in fsfs,<br>
&gt; but in almost every case updated here it is a single call per &#39;svn=
&#39; (or<br>
&gt; other tool) invocation and I don&#39;t think the added complexity and =
the risk<br>
&gt; of accidentally applying it to a pointer in the future makes sense her=
e. The<br>
&gt; cost of a strlen on a string of a few characters certainly isn&#39;t t=
hat high.<br>
&gt;<br>
&gt; The IO overhead is so much larger that global changes like this in the=
 higher<br>
&gt; layers don&#39;t make any sense to me.<br>
<br>
</div>A safer and simpler design to achieve the same goal as this could be:=
<br>
<br>
#define svn_hash_gets(ht, key) \<br>
=C2=A0 svn__internal_hash_get(ht, key, strlen(key))<br>
<br>
which works because the compiler can then optimize out the &quot;strlen&quo=
t; when it is safe and possible to do so.<br></blockquote><div><br></div><d=
iv>Excellent idea. I just tried it and at least GCC is clever enough<br>
to do that optimization. I&#39;ll commit that change tonight.<br></div><br>=
</div>-- Stefan^2.<br></div><div class=3D"gmail_extra"><br>-- <br><font></f=
ont><i>Join one of our <b>free</b> daily demo sessions on</i> <i><b><a href=
=3D"http://www.wandisco.com/training/webinars" target=3D"_blank">Scaling Su=
bversion for the Enterprise</a></b></i><div>

</div><i><p style=3D"margin-top:0in;margin-right:0in;margin-left:0in;margin=
-bottom:0.0001pt;font-size:12pt;font-family:Cambria">
</p></i>
</div></div>

--001a11c1f7a4da3bca04dce7228d--
Received on 2013-05-17 14:16:41 CEST

This is an archived mail posted to the Subversion Dev mailing list.