stef...@apache.org wrote on Mon, May 16, 2011 at 00:02:06 -0000:
> Author: stefan2
> Date: Mon May 16 00:02:05 2011
> New Revision: 1103578
>
> URL: http://svn.apache.org/viewvc?rev=1103578&view=rev
> Log:
> Eliminate unnecessary stat calls during checkout, part 1 of 2.
> Most c/o will be to an otherwise empty directory. In that case,
> we don't need to check for obstructions.
>
> * subversion/include/svn_wc.h
> (svn_wc_get_update_editor4): add clean_checkout parameter
> * subversion/libsvn_wc/deprecated.c
> (svn_wc_get_update_editor3): disable optimization for legacy API
>
> * subversion/libsvn_wc/update_editor.c
> (edit_baton): add clean_checkout flag
> (add_file): skip obstruction checks if that flag has been set
> (make_editor): add clean_checkout parameter and pass it on to baton
> (svn_wc_get_update_editor4): pass clean_checkout through to make_editor
> (svn_wc_get_switch_editor4): disable optimization for switch ops
>
> * subversion/libsvn_client/update.c
> (is_empty_wc): new utility function
> (update_internal): detect applicability of "clean c/o" optimization
> and parametrize update_editor accordingly
>
> Modified:
> subversion/trunk/subversion/include/svn_wc.h
> subversion/trunk/subversion/libsvn_client/update.c
> subversion/trunk/subversion/libsvn_wc/deprecated.c
> subversion/trunk/subversion/libsvn_wc/update_editor.c
>
> Modified: subversion/trunk/subversion/include/svn_wc.h
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_wc.h?rev=1103578&r1=1103577&r2=1103578&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/include/svn_wc.h (original)
> +++ subversion/trunk/subversion/include/svn_wc.h Mon May 16 00:02:05 2011
> @@ -5439,6 +5439,7 @@ svn_wc_get_update_editor4(const svn_delt
> svn_boolean_t allow_unver_obstructions,
> svn_boolean_t adds_as_modification,
> svn_boolean_t server_performs_filtering,
> + svn_boolean_t clean_checkout,
> const char *diff3_cmd,
> const apr_array_header_t *preserved_exts,
> svn_wc_dirents_func_t fetch_dirents_func,
> @@ -5465,8 +5466,8 @@ svn_wc_get_update_editor4(const svn_delt
> * All locks, both those in @a anchor and newly acquired ones, will be
> * released when the editor driver calls @c close_edit.
> *
> - * Always sets @a adds_as_modification to TRUE and @a
> server_performs_filtering
> - * to FALSE.
> + * Always sets @a adds_as_modification to TRUE, @a server_performs_filtering
> + * and @a clean_checkout to FALSE.
> *
> * Uses a svn_wc_conflict_resolver_func_t conflict resolver instead of a
> * svn_wc_conflict_resolver_func2_t.
>
> Modified: subversion/trunk/subversion/libsvn_client/update.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/update.c?rev=1103578&r1=1103577&r2=1103578&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_client/update.c (original)
> +++ subversion/trunk/subversion/libsvn_client/update.c Mon May 16 00:02:05
> 2011
> @@ -92,6 +92,59 @@ svn_client__dirent_fetcher(void *baton,
>
> /*** Code. ***/
>
> +/* Set *CLEAN_CHECKOUT to FALSE only if LOCAL_ABSPATH is a non-empty
> + folder. ANCHOR_ABSPATH is the w/c root and LOCAL_ABSPATH will still
> + be considered empty, if it is equal to ANCHOR_ABSPATH and only
> + contains the admin sub-folder.
> + */
> +static svn_error_t *
> +is_empty_wc(const char *local_abspath,
> + const char *anchor_abspath,
> + svn_boolean_t *clean_checkout,
> + apr_pool_t *pool)
Nit: output variables first?
> +{
> + apr_dir_t *dir;
> + apr_finfo_t finfo;
> + svn_error_t *err;
> +
> + /* "clean" until found dirty */
> + *clean_checkout = TRUE;
> +
> + /* open directory. If it does not exist, yet, a clean one will
> + be created by the caller. If it cannot be openend for other
> + reasons, the caller will detect and report those as well.
Nasty side effect. I don't particularly mind how the function handles
error returns from svn_io_dir_open(), but if it does anything other than
SVN_ERR() then that should be documented in the docstring.
> _+ */
> + err = svn_io_dir_open(&dir, local_abspath, pool);
> + if (err)
> + {
> + svn_error_clear(err);
> + return SVN_NO_ERROR;
> + }
> +
> + for (err = svn_io_dir_read(&finfo, APR_FINFO_NAME, dir, pool);
> + err == SVN_NO_ERROR;
> + err = svn_io_dir_read(&finfo, APR_FINFO_NAME, dir, pool))
> + {
> + /* Ignore entries for this dir and its parent, robustly.
> + (APR promises that they'll come first, so technically
> + this guard could be moved outside the loop. But Ryan Bloom
> + says he doesn't believe it, and I believe him. */
> + if (! (finfo.name[0] == '.'
> + && (finfo.name[1] == '\0'
> + || (finfo.name[1] == '.' && finfo.name[2] == '\0'))))
> + {
> + if ( ! svn_wc_is_adm_dir(finfo.name, pool)
> + || strcmp(local_abspath, anchor_abspath) != 0)
> + {
> + *clean_checkout = FALSE;
> + break;
> + }
> + }
> + }
> +
> + svn_error_clear(err);
Why are you ignoring errors from reading the dir's children?
> + return svn_io_dir_close(dir);
I'm not sure you're even allowed to call this if the previous calls
returned an error. (The documentations are silent AFAICT.)
