subversion-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Paul Burba <ptbu...@gmail.com>
Subject Re: r1414123 - optimize iprops retrieval in libsvn_wc
Date Tue, 12 Feb 2013 17:51:06 GMT
On Wed, Nov 28, 2012 at 2:25 PM, Julian Foad <julianfoad@btopenworld.com> wrote:
>> URL: http://svn.apache.org/viewvc?rev=1414123&view=rev
>
>> Log:
>> In preparation for further performance improvements, make the
>> retrieval of inherited properties use a constant number of db
>> operations.
>
> My comments are about the existing code, not specifically about this revision.
>
>> Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
>> ==============================================================================
>
>> +/* Remove all prop name value pairs from PROP_HASH where the property
>> +   name is not PROPNAME. */
>> +static void
>> +filter_unwanted_props(apr_hash_t *prop_hash,
>> +                      const char * propname,
>> +                      apr_pool_t *scratch_pool)
>> +{
>> +  apr_hash_index_t *hi;
>> +
>> +  for (hi = apr_hash_first(scratch_pool, prop_hash);
>> +       hi;
>> +       hi = apr_hash_next(hi))
>> +    {
>> +      const char *ipropname = svn__apr_hash_index_key(hi);
>> +
>> +      if (strcmp(ipropname, propname) != 0)
>> +        apr_hash_set(prop_hash, ipropname, APR_HASH_KEY_STRING, NULL);
>> +    }
>
> I suppose a much quicker way to select just the wanted key-value pair from the hash is:
>
>   propval = hash_get(propname)
>   hash_clear()

(Julian - sorry for the tardy reply, I was cleaning out old TODOs
today and found this)

apr_hash_clear clears the hash by iterating over it just like we do above:

APR_DECLARE(void) apr_hash_clear(apr_hash_t *ht)
{
    apr_hash_index_t *hi;
    for (hi = apr_hash_first(NULL, ht); hi; hi = apr_hash_next(hi))
        apr_hash_set(ht, hi->this->key, hi->this->klen, NULL);
}

So what do we really gain?  Your suggestion does do away with the
strcmp for every item in the hash, but we gain a call to apr_hash_set.
 I did a quick test: Set 10,000 properties on the root of a WC then
ran 'svn pg some-prop some-subtree-path --show-inherited-props'.

Average execution time over 5 runs was .052s for the existing code and
.056s for your suggested change...so I'll leave the code as is.

-- 
Paul T. Burba
CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development
Skype: ptburba

Mime
View raw message