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

Re: svn commit: r943986 - /subversion/trunk/subversion/libsvn_wc/status.c

From: Daniel Näslund <daniel_at_longitudo.com>
Date: Fri, 14 May 2010 09:26:09 +0200

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
Received on 2010-05-14 09:27:04 CEST

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