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/ svns...

[Ivan Zhakov <iv...@visualsvn.com>]

On Thu, May 16, 2013 at 11:48 PM,  <st...@apache.org> wrote:
> Author: stefan2
> Date: Thu May 16 19:48:47 2013
> New Revision: 1483532
>
> 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.
>
Hi Stefan,

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.

-- 
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com

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/ svns...

[Ivan Zhakov <iv...@visualsvn.com>]

On Fri, May 17, 2013 at 1:40 AM, Stefan Fuhrmann
<st...@wandisco.com> wrote:
>
> On Thu, May 16, 2013 at 11:14 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:
>>
>> On Thu, May 16, 2013 at 11:48 PM,  <st...@apache.org> wrote:
>> > Author: stefan2
>> > Date: Thu May 16 19:48:47 2013
>> > New Revision: 1483532
>> >
>> > 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.
>> >
>> Hi Stefan,
>>
>> 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.
>
>
> It is not essential to use these wrappers in all places. I did that for
> symmetry.
> As for the complexity: The patch should not add any significant maintenance
> costs - no extra parameters etc.
>
It's really easy to create bugs with new API: if someone pass const
char * variable as parameter to svn_hash_sets_fixed_key(). Risk very
high IMHO, while benefits are small.

-- 
Ivan Zhakov
CTO | VisualSVN | http://www.visualsvn.com

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/ svns...

[Stefan Fuhrmann <st...@wandisco.com>]

On Thu, May 16, 2013 at 11:14 PM, Ivan Zhakov <iv...@visualsvn.com> wrote:

> On Thu, May 16, 2013 at 11:48 PM,  <st...@apache.org> wrote:
> > Author: stefan2
> > Date: Thu May 16 19:48:47 2013
> > New Revision: 1483532
> >
> > 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.
> >
> Hi Stefan,
>
> 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.
>

It is not essential to use these wrappers in all places. I did that for
symmetry.
As for the complexity: The patch should not add any significant maintenance
costs - no extra parameters etc.

Not using the new wrappers doesn't break anything. Using them
inappropriately,
e.g. on a const char *, will break result in data not being found in the
hash
(keys would effectively be truncated to 4 or 8 bytes) or even a segfault in
all
uses of that code. So, these cases would caught early on.

-- Stefan^2.

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

*