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

Re: [PATCH]: Avoid extra stat calls during report_revisions on some filesystems

From: Philip Martin <philip_at_codematters.co.uk>
Date: 2005-11-16 20:36:29 CET

Daniel Berlin <dberlin@dberlin.org> writes:

> Index: subversion/libsvn_wc/lock.c
> ===================================================================
> --- subversion/libsvn_wc/lock.c (revision 17369)
> +++ subversion/libsvn_wc/lock.c (working copy)
> @@ -675,10 +675,89 @@ svn_wc_adm_retrieve (svn_wc_adm_access_t
> generally makes the calling code simpler as it doesn't need to check
> for NULL batons. */
> if (! *adm_access)
> - return svn_error_createf (SVN_ERR_WC_NOT_LOCKED, NULL,
> - _("Working copy '%s' is missing or not locked"),
> - svn_path_local_style (path, pool));
> + {
> + const char *wcpath;
> + const svn_wc_entry_t *subdir_entry;
> + svn_node_kind_t wckind;
> + svn_node_kind_t kind;
> + svn_error_t *err;
> +
> + err = svn_wc_entry (&subdir_entry, path, associated, TRUE, pool);
> +
> + /* If we can't get an entry here, we are in pretty bad shape,
> + and will have to fall back to using just regular old paths to
> + see what's going on. */
> + if (err)
> + {
> + err = NULL;

You need to svn_error_clear if you are going to ignore err.

> + subdir_entry = NULL;
> + }
> +
> + err = svn_io_check_path (path, &kind, pool);
> +
> + /* If we can't check the path, we can't make a good error
> + message. */
> + if (err)
> + {
> + svn_error_clear (err);
> + return svn_error_createf (SVN_ERR_WC_NOT_LOCKED, NULL,
> + _("Unable to check path existence for %s"),
> + svn_path_local_style (path, pool));
> + }
> +
> + if (subdir_entry)
> + {
> + if (subdir_entry->kind == svn_node_dir
> + && kind == svn_node_file)
> + {
> + return svn_error_createf (SVN_ERR_WC_NOT_LOCKED, NULL,
> + _("Expected %s to be a directory but found a file"),
> + svn_path_local_style (path, pool));
> + }
> + else if (subdir_entry->kind == svn_node_file
> + && kind == svn_node_dir)
> + {
> + return svn_error_createf (SVN_ERR_WC_NOT_LOCKED, NULL,
> + _("Expected %s to be a file but found a directory"),
> + svn_path_local_style (path, pool));
> + }
> + }
> +
> + wcpath = svn_wc__adm_path (path, FALSE, pool, NULL);
> + err = svn_io_check_path (wcpath, &wckind, pool);

That's the second new stat() call in a function that previously did no
disk IO at all :-( You may have improved update but have other
operations have been penalised?

At the very least you should use the result of the first stat() to
avoid doing the second stat() when it must fail. The access baton
stuff already has support for "missing" directories, can you use that
to avoid some of the stat() calls?

> +
> + /* If we can't check the path, we can't make a good error
> + message. */
> + if (err)
> + {
> + svn_error_clear (err);
> + return svn_error_createf (SVN_ERR_WC_NOT_LOCKED, NULL,
> + _("Unable to check path existence for %s"),
> + svn_path_local_style (wcpath, pool));
> + }
> +
> + if (kind == svn_node_none)
> + return svn_error_createf (SVN_ERR_WC_NOT_LOCKED, NULL,
> + _("Directory '%s' is missing"),
> + svn_path_local_style (path, pool));
> +
> + else if (kind == svn_node_dir && wckind == svn_node_none)
> + return svn_error_createf (SVN_ERR_WC_NOT_LOCKED, NULL,
> + _("Directory '%s' containing working copy admin area is missing"),
> + svn_path_local_style (wcpath, pool));
> +
> + else if (kind == svn_node_dir && wckind == svn_node_dir)
> + return svn_error_createf (SVN_ERR_WC_NOT_LOCKED, NULL,
> + _("Unable to lock %s"),
> + svn_path_local_style (path, pool));
> +
> + /* If all else fails, return our useless generic error. */
> + return svn_error_createf (SVN_ERR_WC_NOT_LOCKED, NULL,
> + _("Working copy '%s' is not locked"),
> + svn_path_local_style (path, pool));
>
> +
> + }
> return SVN_NO_ERROR;
> }

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Nov 16 20:37:18 2005

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.