On Fri, Feb 11, 2011 at 8:18 AM, Bert Huijben <bert_at_qqmail.nl> wrote:
>
>
>> -----Original Message-----
>> From: hwright_at_apache.org [mailto:hwright_at_apache.org]
>> Sent: donderdag 10 februari 2011 23:56
>> To: commits_at_subversion.apache.org
>> Subject: svn commit: r1069602 -
>> /subversion/trunk/subversion/libsvn_wc/wc_db.c
>>
>> Author: hwright
>> Date: Thu Feb 10 22:56:11 2011
>> New Revision: 1069602
>>
>> URL: http://svn.apache.org/viewvc?rev=1069602&view=rev
>> Log:
>> Make the flush_entries() function fetch its own PDH, allowing callers
>> to avoid
>> fetching it.
>>
>> This has the side effect of causing flush_entries() to flush both the
>> current,
>> as well as the parent's caches in all cases. But since that will only
>> happen
>> in the backward compat scenario, it will have neglible real performance
>> impact.
>>
>> * subversion/libsvn_wc/wc_db.c
>> (flush_entries): Don't take a PDH param, fetch one internally.
>> [elsewhere]: Adjust callers to not provide a PDH.
>>
>> Modified:
>> subversion/trunk/subversion/libsvn_wc/wc_db.c
>>
>> Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
>> URL:
>> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_d
>> b.c?rev=1069602&r1=1069601&r2=1069602&view=diff
>> =======================================================================
>> =======
>> --- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
>> +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Thu Feb 10 22:56:11
>> 2011
>> @@ -1057,10 +1057,16 @@ gather_repo_children(const apr_array_hea
>> /* */
>> static svn_error_t *
>> flush_entries(svn_wc__db_t *db,
>> - svn_wc__db_pdh_t *pdh,
>> const char *local_abspath,
>> apr_pool_t *scratch_pool)
>> {
>> + svn_wc__db_pdh_t *pdh;
>> + const char *local_relpath;
>> +
>> + SVN_ERR(svn_wc__db_pdh_parse_local_abspath(&pdh, &local_relpath, db,
>> + local_abspath,
>> svn_sqlite__mode_readwrite,
>> + scratch_pool, scratch_pool));
>> +
>> if (pdh->adm_access)
>> svn_wc__adm_access_set_entries(pdh->adm_access, NULL);
>
> This code used to be just a few pointer compares in the normal case of no cached adm_access instances, but it is now splitting an abspath to a local relpath (read: allocating ram a few times) via several hashtable lookups. In some cases it might even perform disk-io. (By statting if a dirent is a file or a directory)
>
> Calling svn_wc__db_pdh_parse_local_abspath is not that cheap.
>
> I think we should consider reverting this change and maybe add a helper which takes a local_relpath instead for the cases where we already have that available.
The endgame here is to not use PDHs at all the adm_access baton
caching. Instead, I plan to have a simple hash with local_abspaths
indexing the batons. As long as we don't mind being overly aggressive
in clearing the cache, we won't need to parse the PDH (thus incurring
the extra stat()) to flush the cache.
Another, rather simple, strategy is to have a 'backward_compat_mode'
flag in the wcroot which is only set when entering libsvn_wc via one
of the legacy APIs. We could then easily check the flag for an early
out in flush_entries().
Either way, we won't be parsing the pdh for long in flush_entries().
-Hyrum
Received on 2011-02-11 14:42:31 CET