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; stef...@apache.org; dev@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 <julianf...@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.fuhrm...@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:julianf...@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:b...@qqmail.nl";>Bert Huijben</A>; = <A href=3D"mailto:i...@visualsvn.com";>Ivan Zhakov</A>; <A href=3D"mailto:st= ef...@apache.org">stef...@apache.org</A>; <A href=3D"mailto:dev@subversion.= apache.org">dev@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"><<a href=3D"mailto:julianfoad@= btopenworld.com" target=3D"_blank">julianf...@btopenworld.com</a>></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> > Ivan Zhakov wrote:<br> >>> Author: stefan2<br> <div class=3D"im">>>> URL: <a href=3D"http://svn.apache.org/r14835= 32" target=3D"_blank">http://svn.apache.org/r1483532</a><br> >>> Log:<br> >>> We frequently use property name constants in conjunction with = hash<br> >>> containers. Provide new wrappers around apr_hash_get and apr_h= ash_set<br> >>> that accept such string constants and statically determine the= ir size.<br> >>> That minimizes the hash access costs.<br> >>><br> >>> Mass change hash get and set calls for SVN_PROP_* constants.<b= r> >><br> </div><div class=3D"im">>> Is the performance gain costs code complex= ity? Please understand my<br> >> correctly: it's great improve Subversion speed. I just don'= ;t like<br> >> the idea getting code more complicated to win just several cycles.= <br> ><br> > I can see this change making some difference when used in a tight loop= in fsfs,<br> > but in almost every case updated here it is a single call per 'svn= ' (or<br> > other tool) invocation and I don't think the added complexity and = the risk<br> > of accidentally applying it to a pointer in the future makes sense her= e. The<br> > cost of a strlen on a string of a few characters certainly isn't t= hat high.<br> ><br> > The IO overhead is so much larger that global changes like this in the= higher<br> > layers don'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 "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'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--
