On 17.05.2011 09:51, Daniel Shahaf wrote:
> stefan2_at_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?
Yup. r1104306 should fix all the issues you identified here.
>> +{
>> + 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.
That and I changed the behavior to return "unclear"
whenever there is an unexpected error.
>> _+ */
>> + 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?
My mistake.
>> + 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.)
It is legal. If it would be illegal on the APR level, svn_io_dir_read
would need to be patched to close the handle after string conversion
failures as well.
-- Stefan^2.
Received on 2011-05-17 17:01:35 CEST