> -----Original Message-----
> From: stefan2_at_apache.org [mailto:stefan2_at_apache.org]
> Sent: maandag 16 mei 2011 3:07
> To: commits_at_subversion.apache.org
> Subject: svn commit: r1103589 - in /subversion/trunk/subversion/libsvn_wc:
> wc_db.c wc_db_pristine.c wc_db_private.h wc_db_wcroot.c
>
> Author: stefan2
> Date: Mon May 16 01:07:11 2011
> New Revision: 1103589
>
> URL: http://svn.apache.org/viewvc?rev=1103589&view=rev
> Log:
> Eliminate unnecessary stat calls during checkout, part 2 of 2.
>
> If provided as parameter, svn_wc__db_wcroot_parse_local_abspath
> does not need to use a stat call to detect the kind of node
> behind the given abspath. For many *frequent* callers that
> is known.
>
> * subversion/libsvn_wc/wc_db_private.h
> (svn_wc__db_wcroot_parse_local_abspath): add kind parameter
> * subversion/libsvn_wc/wc_db_wcroot.c
> (svn_wc__db_wcroot_parse_local_abspath): stat for kind of
> if it is not already known
Why do you use svn_node_kind_t instead of svn_wc__db_kind_t?
The changes to svn_wc__db_base_*() you applied all assume that the working copy files match what the functions receive, while in general you can't assume that. Once a node is shadowed by a WORKING node, or by some unversioned obstruction all bets are off. That is what this function tested for.
In the old entries-world, you would never get an access baton for a directory that was obstructed this way, so this simply can't happen. And now we have to check for that in another way.
This parse path function does that for us in a lot of places (and probably in more places than it should), but we can't avoid it in all cases as ignoring it one time to many will just break both working copies. (It just overwrites the inner, with information from the outer. So both will be out of sync).
Summarized:
What the DB thinks there should be in a specific layer doesn't have to match what is in the working copy.
> * subversion/libsvn_wc/wc_db_pristine.c
> (svn_wc__db_pristine_get_path, svn_wc__db_pristine_read,
> svn_wc__db_pristine_get_tempdir, svn_wc__db_pristine_install,
> svn_wc__db_pristine_get_md5, svn_wc__db_pristine_get_sha1,
> svn_wc__db_pristine_remove, svn_wc__db_pristine_cleanup,
> svn_wc__db_pristine_check): don't use the optimization
All these have a wri_abspath, so you can't use the optimization. A wri_abspath can be any path in a working copy (versioned or unversioned).
> * subversion/libsvn_wc/wc_db.c
> (kind_by_filesize): new utility function
About svn_wc__db_global_record_fileinfo:
This function is only called for files... Why perform a check to guess if a node is maybe a directory?
Directories don't have a valid recorded_*; never had.
>
> (get_statement_for_path, svn_wc__db_to_relpath,
> svn_wc__db_from_relpath, svn_wc__db_get_wcroot,
> svn_wc__db_base_add_symlink,
> add_absent_excluded_not_present_node,
> svn_wc__db_base_get_info, svn_wc__db_base_get_children,
> svn_wc__db_base_clear_dav_cache_recursive,
> svn_wc__db_external_add_file,
> svn_wc__db_external_add_symlink, svn_wc__db_external_remove,
> svn_wc__db_external_read, svn_wc__db_external_read_pristine_props,
> svn_wc__db_external_read_props, svn_wc__db_op_copy,
> svn_wc__db_op_copy_shadowed_layer, svn_wc__db_op_copy_file,
> svn_wc__db_op_copy_symlink, svn_wc__db_op_add_symlink,
> svn_wc__db_op_set_props, svn_wc__db_op_set_changelist,
> svn_wc__db_op_mark_resolved, svn_wc__db_op_set_tree_conflict,
> svn_wc__db_op_revert, svn_wc__db_revert_list_read,
> svn_wc__db_revert_list_notify, svn_wc__db_op_read_all_tree_conflicts,
> svn_wc__db_op_read_tree_conflict, svn_wc__db_op_remove_node,
> svn_wc__db_temp_op_remove_working, svn_wc__db_op_delete,
> svn_wc__db_read_info, svn_wc__db_read_pristine_info,
> svn_wc__db_read_node_install_info, svn_wc__db_read_url,
> svn_wc__db_read_props, svn_wc__db_read_props_streamily,
> svn_wc__db_read_pristine_props,
> svn_wc__db_read_children_of_working_node,
> svn_wc__db_node_check_replace, svn_wc__db_read_children,
> svn_wc__db_global_commit, svn_wc__db_global_update,
> svn_wc__db_op_bump_revisions_post_update, svn_wc__db_lock_add,
> svn_wc__db_lock_remove, svn_wc__db_scan_base_repos,
> svn_wc__db_scan_addition, svn_wc__db_scan_deletion,
> svn_wc__db_wq_add, svn_wc__db_wq_fetch,
> svn_wc__db_wq_completed,
> svn_wc__db_read_conflict_victims, svn_wc__db_read_conflicts,
> svn_wc__db_read_kind, svn_wc__db_node_hidden,
> svn_wc__db_is_wcroot,
> svn_wc__db_temp_wcroot_tempdir, svn_wc__db_wclock_obtain,
> svn_wc__db_wclock_release, svn_wc__db_wclock_owns_lock,
> svn_wc__db_temp_op_start_directory_update,
> svn_wc__db_temp_op_make_copy,
> svn_wc__db_temp_op_set_text_conflict_marker_files,
> svn_wc__db_temp_op_set_property_conflict_marker_file,
> svn_wc__db_temp_op_set_new_dir_to_incomplete,
> svn_wc__db_info_below_working,
> svn_wc__db_get_not_present_descendants,
> svn_wc__db_min_max_revisions, svn_wc__db_is_sparse_checkout,
> svn_wc__db_has_switched_subtrees, svn_wc__db_get_absent_subtrees,
> svn_wc__db_has_local_mods, svn_wc__db_revision_status,
> svn_wc__db_base_get_lock_tokens_recursive, svn_wc__db_verify):
> don't use the optimization
>
> (svn_wc__db_base_add_directory, svn_wc__db_base_get_children_info,
> svn_wc__db_external_add_dir, svn_wc__db_op_copy_dir,
> svn_wc__db_op_add_directory, svn_wc__db_op_set_base_depth,
> svn_wc__db_read_children_info,
> svn_wc__db_read_children_walker_info,
> svn_wc__db_global_relocate, svn_wc__db_temp_get_format,
> svn_wc__db_temp_get_access, svn_wc__db_temp_set_access,
> svn_wc__db_temp_close_access, svn_wc__db_temp_clear_access,
> svn_wc__db_temp_borrow_sdb,
> svn_wc__db_temp_op_end_directory_update):
> path parameters are known to be directories
'when they apply to existing directories on disk', which is generally unknown. This moves the requirement to check that to the caller, which is not always a safe assumption.
>
> (svn_wc__db_base_add_file, svn_wc__db_op_add_file):
> path parameters are known to be files
The update editor has no problem adding a base node under an existing WORKING directory (aka 'A shadowed update'), so the reason why is not correct. But I think adds are performed below a parent, so you can just assume file for that reason.
(But maybe this would need a better argument to match that instead).
> (svn_wc__db_external_record_fileinfo,
> svn_wc__db_global_record_fileinfo):
> path parameters are files, if the file length is > 0.
Did you get any calls for directories here?
I really hope you didn't.
Bert
Received on 2011-05-16 08:09:48 CEST