[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

RE: svn commit: r1069602 - /subversion/trunk/subversion/libsvn_wc/wc_db.c

From: Bert Huijben <bert_at_qqmail.nl>
Date: Fri, 11 Feb 2011 09:18:18 +0100

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

        Bert
Received on 2011-02-11 09:19:00 CET

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.