You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by da...@apache.org on 2010/05/13 21:32:11 UTC
svn commit: r943986 - /subversion/trunk/subversion/libsvn_wc/status.c
Author: dannas
Date: Thu May 13 19:32:11 2010
New Revision: 943986
URL: http://svn.apache.org/viewvc?rev=943986&view=rev
Log:
Don't use svn_wc_entry for fetching an url.
* subversion/libsvn_wc/status.c
(assemble_status): Fetch the url with a node func. Use
svn_wc__check_wc_root() for determining if a path is switched.
Modified:
subversion/trunk/subversion/libsvn_wc/status.c
Modified: subversion/trunk/subversion/libsvn_wc/status.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/status.c?rev=943986&r1=943985&r2=943986&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_wc/status.c (original)
+++ subversion/trunk/subversion/libsvn_wc/status.c Thu May 13 19:32:11 2010
@@ -284,6 +284,7 @@ assemble_status(svn_wc_status3_t **statu
svn_wc_status3_t *stat;
svn_wc__db_status_t db_status;
svn_wc__db_kind_t db_kind;
+ const char *url;
svn_boolean_t locked_p = FALSE;
svn_boolean_t switched_p = FALSE;
const svn_wc_conflict_description2_t *tree_conflict;
@@ -320,25 +321,22 @@ assemble_status(svn_wc_status3_t **statu
&lock, db, local_abspath, result_pool,
scratch_pool));
+ SVN_ERR(svn_wc__internal_node_get_url(&url, db, local_abspath,
+ result_pool, scratch_pool));
+
/** File externals are switched files, but they are not shown as
such. To be switched it must have both an URL and a parent with
- an URL, at the very least. If this is the root folder on the
- (virtual) disk, entry and parent_entry will be equal. */
+ an URL, at the very least. */
if (entry->file_external_path)
{
file_external_p = TRUE;
}
- else if (entry->url && parent_entry && parent_entry->url &&
- entry != parent_entry)
+ else
{
- /* An item is switched if:
- parent-url + basename(path) != entry->url */
- switched_p = (strcmp(
- svn_uri_join(parent_entry->url,
- svn_path_uri_encode(svn_dirent_basename(
- local_abspath, NULL),
- scratch_pool),
- scratch_pool), entry->url) != 0);
+ svn_boolean_t is_wc_root; /* Not used. */
+
+ SVN_ERR(svn_wc__check_wc_root(&is_wc_root, NULL, &switched_p, db,
+ local_abspath, scratch_pool));
}
/* Examine whether our directory metadata is present, and compensate
@@ -603,7 +601,7 @@ assemble_status(svn_wc_status3_t **statu
stat->file_external = file_external_p;
stat->copied = entry->copied;
stat->repos_lock = repos_lock;
- stat->url = (entry->url ? entry->url : NULL);
+ stat->url = url;
stat->revision = revision;
stat->changed_rev = changed_rev;
stat->changed_author = changed_author;
Re: svn commit: r943986 -
/subversion/trunk/subversion/libsvn_wc/status.c
Posted by Daniel Näslund <da...@longitudo.com>.
On Thu, May 13, 2010 at 10:35:36PM +0200, Bert Huijben wrote:
>
> > Log:
> > Don't use svn_wc_entry for fetching an url.
> >
> > * subversion/libsvn_wc/status.c
> > (assemble_status): Fetch the url with a node func. Use
> > svn_wc__check_wc_root() for determining if a path is switched.
> >
[...]
> > + SVN_ERR(svn_wc__internal_node_get_url(&url, db, local_abspath,
> > + result_pool, scratch_pool));
> > +
> > /** File externals are switched files, but they are not shown as
> > such. To be switched it must have both an URL and a parent with
> > - an URL, at the very least. If this is the root folder on the
> > - (virtual) disk, entry and parent_entry will be equal. */
> > + an URL, at the very least. */
> > if (entry->file_external_path)
> > {
> > file_external_p = TRUE;
> > }
> > - else if (entry->url && parent_entry && parent_entry->url &&
> > - entry != parent_entry)
> > + else
> > {
> > - /* An item is switched if:
> > - parent-url + basename(path) != entry->url */
> > - switched_p = (strcmp(
> > - svn_uri_join(parent_entry->url,
> > - svn_path_uri_encode(svn_dirent_basename(
> > - local_abspath, NULL),
> > - scratch_pool),
> > - scratch_pool), entry->url) != 0);
> > + svn_boolean_t is_wc_root; /* Not used. */
> > +
> > + SVN_ERR(svn_wc__check_wc_root(&is_wc_root, NULL, &switched_p,
> > db,
> > + local_abspath, scratch_pool));
>
> You replace a few in memory operations with several database
> operations on the node and its parent(s) here. Was this intended? (I
> don't think this will make status faster). While walking the tree for
> status you know the url of the parent and of the node itself.
>
> This check_wc_root function starts looking in the database for the url
> of the node.. and its parent (probably inherited, so looking further
> up)... and then does this same check for you.
Pasting my answer to the same question from #svn-dev:
09:17 <@dannas> Bert: Yeah, the url-fetching and switch detection is
sub-optimal at the moment. I just set out to replace the entry calls,
thinking I could optimize for performance later but I should have
atleast written a TODO about it,
09:17 <@dannas> If we call assemble_status() from
send_status_structure() with my next patch for removing entry->url usage
there we may end up fetching the url for the same path three times! :)
09:18 <@dannas> I'll loook into you suggestion later today, it makes a
lot of sense.
09:20 <@dannas> The only thing I need to add is a way to detect if our
path is wc_root.
09:21 <@dannas> It was the problem to detect a wc-root that lead me to
use the bulky svn_wc__check_wc_root()
Daniel
RE: svn commit: r943986 - /subversion/trunk/subversion/libsvn_wc/status.c
Posted by Bert Huijben <be...@vmoo.com>.
> -----Original Message-----
> From: dannas@apache.org [mailto:dannas@apache.org]
> Sent: donderdag 13 mei 2010 21:32
> To: commits@subversion.apache.org
> Subject: svn commit: r943986 -
> /subversion/trunk/subversion/libsvn_wc/status.c
>
> Author: dannas
> Date: Thu May 13 19:32:11 2010
> New Revision: 943986
>
> URL: http://svn.apache.org/viewvc?rev=943986&view=rev
> Log:
> Don't use svn_wc_entry for fetching an url.
>
> * subversion/libsvn_wc/status.c
> (assemble_status): Fetch the url with a node func. Use
> svn_wc__check_wc_root() for determining if a path is switched.
>
> Modified:
> subversion/trunk/subversion/libsvn_wc/status.c
>
> Modified: subversion/trunk/subversion/libsvn_wc/status.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/stat
> us.c?rev=943986&r1=943985&r2=943986&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_wc/status.c (original)
> +++ subversion/trunk/subversion/libsvn_wc/status.c Thu May 13 19:32:11
> +++ 2010
> @@ -284,6 +284,7 @@ assemble_status(svn_wc_status3_t **statu
> svn_wc_status3_t *stat;
> svn_wc__db_status_t db_status;
> svn_wc__db_kind_t db_kind;
> + const char *url;
> svn_boolean_t locked_p = FALSE;
> svn_boolean_t switched_p = FALSE;
> const svn_wc_conflict_description2_t *tree_conflict; @@ -320,25 +321,22
> @@ assemble_status(svn_wc_status3_t **statu
> &lock, db, local_abspath, result_pool,
> scratch_pool));
>
> + SVN_ERR(svn_wc__internal_node_get_url(&url, db, local_abspath,
> + result_pool, scratch_pool));
> +
> /** File externals are switched files, but they are not shown as
> such. To be switched it must have both an URL and a parent with
> - an URL, at the very least. If this is the root folder on the
> - (virtual) disk, entry and parent_entry will be equal. */
> + an URL, at the very least. */
> if (entry->file_external_path)
> {
> file_external_p = TRUE;
> }
> - else if (entry->url && parent_entry && parent_entry->url &&
> - entry != parent_entry)
> + else
> {
> - /* An item is switched if:
> - parent-url + basename(path) != entry->url */
> - switched_p = (strcmp(
> - svn_uri_join(parent_entry->url,
> - svn_path_uri_encode(svn_dirent_basename(
> - local_abspath, NULL),
> - scratch_pool),
> - scratch_pool), entry->url) != 0);
> + svn_boolean_t is_wc_root; /* Not used. */
> +
> + SVN_ERR(svn_wc__check_wc_root(&is_wc_root, NULL, &switched_p,
> db,
> + local_abspath, scratch_pool));
You replace a few in memory operations with several database operations on the node and its parent(s) here. Was this intended? (I don't think this will make status faster).
While walking the tree for status you know the url of the parent and of the node itself.
This check_wc_root function starts looking in the database for the url of the node.. and its parent (probably inherited, so looking further up)... and then does this same check for you.
Bert
> }
>
> /* Examine whether our directory metadata is present, and compensate
> @@ -603,7 +601,7 @@ assemble_status(svn_wc_status3_t **statu
> stat->file_external = file_external_p;
> stat->copied = entry->copied;
> stat->repos_lock = repos_lock;
> - stat->url = (entry->url ? entry->url : NULL);
> + stat->url = url;
> stat->revision = revision;
> stat->changed_rev = changed_rev;
> stat->changed_author = changed_author;
>