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

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.

- Julian
Received on 2013-05-17 00:25:42 CEST