Re: r1414123 - optimize iprops retrieval in libsvn_wc

On Wed, Nov 28, 2012 at 2:25 PM, Julian Foad <julianfoad_at_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