So as a followup to the restartable checkouts thing, I went and
implemented the new checkout system. You know, the one where
svn_client_checkout() simply creates an incomplete root-wc-dir, and
then runs svn_client_update() on it? Everybody loved this idea. It
meant losing a lot of RA code.
The good news: it seems to work exactly as expected. It was a simple
little change.
The bad news: for both of our network layers, this new system slows
down checkouts by ~35%. I did a bunch of timings, checking out
subversion's own /trunk directory via http://localhost,
svn://localhost, and file:///. For http: and svn:, the new system
took about 35% longer. For file:, the new system was *faster* by
about 35%. Crazy.
I don't know what to think... or whether to apply the patch. I'll
attach it to this mail and lets others examine it, analyze it, etc.
(If it ever gets applied, it means we can follow-up with a patch that
tosses all the RA->checkout implementations.)
----------------------------------------------------------------
A new checkout implementation: don't call RA->checkout, but instead
have svn_client_checkout() secretly run svn_client_update() on an
incomplete wc-root directory.
* cmdline/checkout-cmd.c (svn_cl__checkout): don't ever pass an
unspecified revision into svn_client_checkout; default to HEAD instead.
* svn_wc.h (svn_wc_ensure_adm): declare as a public function.
* libsvn_wc/adm_files.h (svn_wc__ensure_adm): remove declaration.
* libsvn_wc/adm_files.c (svn_wc_ensure_adm): was svn_wc__ensure_adm.
* libsvn_wc/adm_ops.c (svn_wc_add): update caller.
* libsvn_wc/update_editor.c (prep directory): update caller.
* libsvn_client/checkout.c (svn_client__checkout_internal): fulfill
docstring promise; opt_revision_t passed in mustn't be
'unspecified'. Also, instead of calling RA->checkout(), create a
new incomplete root wc-dir and run svn_client_update on it.
Index: subversion/include/svn_wc.h
===================================================================
--- subversion/include/svn_wc.h (revision 5955)
+++ subversion/include/svn_wc.h (working copy)
@@ -773,6 +773,28 @@
apr_pool_t *pool);
+
+/* Ensure that an administrative area exists for PATH, so that PATH is a
+ * working copy subdir based on URL at REVISION.
+ *
+ * If the administrative area does not exist then it will be created and
+ * initialized to a unlocked state.
+ *
+ * If the administrative area already exists then the given URL must match
+ * the URL in the administrative area or an error will be returned. The
+ * given REVISION must also match except for the special case of adding a
+ * directory that has a name matching one scheduled for deletion, in which
+ * case REVISION must be zero.
+ *
+ * Do not ensure existence of PATH itself; if PATH does not exist,
+ * return error.
+ */
+svn_error_t *svn_wc_ensure_adm (const char *path,
+ const char *url,
+ svn_revnum_t revision,
+ apr_pool_t *pool);
+
+
/**
* @defgroup svn_wc_status working copy status.
Index: subversion/libsvn_wc/adm_ops.c
===================================================================
--- subversion/libsvn_wc/adm_ops.c (revision 5955)
+++ subversion/libsvn_wc/adm_ops.c (working copy)
@@ -1040,7 +1040,7 @@
/* Make sure this new directory has an admistrative subdirectory
created inside of it */
- SVN_ERR (svn_wc__ensure_adm (path, new_url, 0, pool));
+ SVN_ERR (svn_wc_ensure_adm (path, new_url, 0, pool));
}
else
{
@@ -1048,8 +1048,8 @@
the admin directory already in existance, then the dir will
contain the copyfrom settings. So we need to pass the
copyfrom arguments to the ensure call. */
- SVN_ERR (svn_wc__ensure_adm (path, copyfrom_url,
- copyfrom_rev, pool));
+ SVN_ERR (svn_wc_ensure_adm (path, copyfrom_url,
+ copyfrom_rev, pool));
}
/* We want the locks to persist, so use the access baton's pool */
Index: subversion/libsvn_wc/adm_files.c
===================================================================
--- subversion/libsvn_wc/adm_files.c (revision 5955)
+++ subversion/libsvn_wc/adm_files.c (working copy)
@@ -1100,10 +1100,10 @@
svn_error_t *
-svn_wc__ensure_adm (const char *path,
- const char *url,
- svn_revnum_t revision,
- apr_pool_t *pool)
+svn_wc_ensure_adm (const char *path,
+ const char *url,
+ svn_revnum_t revision,
+ apr_pool_t *pool)
{
svn_boolean_t exists_already;
Index: subversion/libsvn_wc/update_editor.c
===================================================================
--- subversion/libsvn_wc/update_editor.c (revision 5955)
+++ subversion/libsvn_wc/update_editor.c (working copy)
@@ -467,8 +467,8 @@
/* Make sure it's the right working copy, either by creating it so,
or by checking that it is so already. */
- SVN_ERR (svn_wc__ensure_adm (db->path, ancestor_url, ancestor_revision,
- pool));
+ SVN_ERR (svn_wc_ensure_adm (db->path, ancestor_url, ancestor_revision,
+ pool));
if (! db->edit_baton->adm_access
|| strcmp (svn_wc_adm_access_path (db->edit_baton->adm_access),
Index: subversion/libsvn_wc/adm_files.h
===================================================================
--- subversion/libsvn_wc/adm_files.h (revision 5955)
+++ subversion/libsvn_wc/adm_files.h (working copy)
@@ -222,28 +222,6 @@
svn_boolean_t wcprops,
apr_pool_t *pool);
-
-/* Ensure that an administrative area exists for PATH, so that PATH is a
- * working copy subdir based on URL at REVISION.
- *
- * If the administrative area does not exist then it will be created and
- * initialized to a unlocked state.
- *
- * If the administrative area already exists then the given URL must match
- * the URL in the administrative area or an error will be returned. The
- * given REVISION must also match except for the special case of adding a
- * directory that has a name matching one scheduled for deletion, in which
- * case REVISION must be zero.
- *
- * Do not ensure existence of PATH itself; if PATH does not exist,
- * return error.
- */
-svn_error_t *svn_wc__ensure_adm (const char *path,
- const char *url,
- svn_revnum_t revision,
- apr_pool_t *pool);
-
-
/* Blow away the admistrative directory associated with the access baton
ADM_ACCESS. This closes ADM_ACCESS, but it is safe to close ADM_ACCESS
again, after calling this function. */
Index: subversion/libsvn_client/checkout.c
===================================================================
--- subversion/libsvn_client/checkout.c (revision 5955)
+++ subversion/libsvn_client/checkout.c (working copy)
@@ -63,6 +63,13 @@
assert (path != NULL);
assert (URL != NULL);
+ /* Fulfill the docstring promise of svn_client_checkout: */
+ if ((revision->kind != svn_opt_revision_number)
+ && (revision->kind != svn_opt_revision_date)
+ && (revision->kind != svn_opt_revision_head))
+ return svn_error_create (SVN_ERR_CLIENT_BAD_REVISION, NULL,
+ "Bogus revision passed to svn_client_checkout");
+
/* Get revnum set to something meaningful, so we can fetch the
checkout editor. */
if (revision->kind == svn_opt_revision_number)
@@ -88,7 +95,7 @@
&checkout_edit_baton,
traversal_info,
pool));
-
+
{
void *ra_baton, *session;
svn_ra_plugin_t *ra_lib;
@@ -107,15 +114,15 @@
SVN_ERR (svn_client__get_revision_number
(&revnum, ra_lib, session, revision, path, pool));
- /* Tell RA to do a checkout of REVISION; if we pass an invalid
- revnum, that means RA will fetch the latest revision. */
- err = ra_lib->do_checkout (session,
- revnum,
- recurse,
- checkout_editor,
- checkout_edit_baton,
- pool);
+ /* Bootstrap: create an incomplete working-copy root dir. Its
+ entries file should only have an entry for THIS_DIR with a
+ URL, revnum, and an 'incomplete' flag. */
+ SVN_ERR (svn_io_make_dir_recursively (path, pool));
+ SVN_ERR (svn_wc_ensure_adm (path, URL, revnum, pool));
+ /* Have update flesh everything out. */
+ err = svn_client_update (path, revision, recurse, ctx, pool);
+
if (err)
{
/* Don't rely on the error handling to handle the sleep later, do
Index: subversion/clients/cmdline/checkout-cmd.c
===================================================================
--- subversion/clients/cmdline/checkout-cmd.c (revision 5955)
+++ subversion/clients/cmdline/checkout-cmd.c (working copy)
@@ -130,6 +130,11 @@
target_dir = svn_path_join (local_dir, target_dir, subpool);
}
+ /* svn_client_checkout() doesn't accept an unspecified revision,
+ so default to HEAD. */
+ if (opt_state->start_revision.kind == svn_opt_revision_unspecified)
+ opt_state->start_revision.kind = svn_opt_revision_head;
+
SVN_ERR (svn_client_checkout (repos_url,
target_dir,
&(opt_state->start_revision),
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Mon May 19 18:28:54 2003