On Thu, Mar 7, 2013 at 10:46 AM, <rhuij...@apache.org> wrote: > Author: rhuijben > Date: Thu Mar 7 15:46:08 2013 > New Revision: 1453925 > > URL: http://svn.apache.org/r1453925 > Log: > Handle 'svn:ignore' and 'svn:global-ignores' values as a single list for > status processing and for svn_wc_get_ignores2(). > > This patch simplifies some status code and makes a very expensive check per > directory that contains unversioned files cheaper. > > On my profile working copy with unversioned files in every directory, but > no local changes this accounts for a +- 6% performance improvement on > 'svn status' > (The amount of CPU time spend on obtaining these properties was reduced by > 50%) > > * subversion/libsvn_wc/props.c > (svn_wc__get_iprops): Update caller. > > * subversion/libsvn_wc/status.c > (collect_ignore_patterns): Obtain a single list of patterns. Remove > had_props > optimization as that doesn't help for inherited properties anyway. > (send_unversioned_item, > one_child_status, > get_dir_status): Remove inherited specialized code. > > (svn_wc_get_ignores2): Update code to really obtain all ignore patterns, as > documented. > > * subversion/libsvn_wc/wc_db.c > (db_read_inherited_props): Add actual_props argument. Constify output array. > Use db query directly to calculate whether the parent is what it should be > and inherited property eligability with a single query. Obtain actual > props > if requested. > Note that we are not really interested in switched paths here, but just if > the parent directory is the parent of the child we are obtaining > information > for. > > (svn_wc__db_read_inherited_props): Obtain actual_props if requested. > > * subversion/libsvn_wc/wc_db.h > (svn_wc__db_read_inherited_props): Update documentation and prototype. > > Modified: > subversion/trunk/subversion/libsvn_wc/props.c > subversion/trunk/subversion/libsvn_wc/status.c > subversion/trunk/subversion/libsvn_wc/wc_db.c > subversion/trunk/subversion/libsvn_wc/wc_db.h > > Modified: subversion/trunk/subversion/libsvn_wc/props.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/props.c?rev=1453925&r1=1453924&r2=1453925&view=diff > ============================================================================== > --- subversion/trunk/subversion/libsvn_wc/props.c (original) > +++ subversion/trunk/subversion/libsvn_wc/props.c Thu Mar 7 15:46:08 2013 > @@ -2314,7 +2314,7 @@ svn_wc__get_iprops(apr_array_header_t ** > apr_pool_t *scratch_pool) > { > return svn_error_trace( > - svn_wc__db_read_inherited_props(inherited_props, > + svn_wc__db_read_inherited_props(inherited_props, NULL, > wc_ctx->db, local_abspath, > propname, > result_pool, scratch_pool)); > > Modified: subversion/trunk/subversion/libsvn_wc/status.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/status.c?rev=1453925&r1=1453924&r2=1453925&view=diff > ============================================================================== > --- subversion/trunk/subversion/libsvn_wc/status.c (original) > +++ subversion/trunk/subversion/libsvn_wc/status.c Thu Mar 7 15:46:08 2013 > @@ -962,17 +962,16 @@ send_status_structure(const struct walk_ > */ > static svn_error_t * > collect_ignore_patterns(apr_array_header_t **patterns, > - apr_array_header_t **inherited_patterns, > svn_wc__db_t *db, > const char *local_abspath, > const apr_array_header_t *ignores, > - svn_boolean_t may_have_props, > apr_pool_t *result_pool, > apr_pool_t *scratch_pool) > { > int i; > - const svn_string_t *value; > - apr_hash_t *props = NULL; > + apr_hash_t *props; > + const apr_array_header_t *inherited_props; > + svn_error_t *err; > > /* ### assert we are passed a directory? */ > > @@ -986,56 +985,46 @@ collect_ignore_patterns(apr_array_header > ignore); > } > > - if (may_have_props) > + err = svn_wc__db_read_inherited_props(&inherited_props, &props, > + db, local_abspath, > + SVN_PROP_INHERITABLE_IGNORES, > + scratch_pool, scratch_pool); > + > + if (err) > { > - /* Add any svn:ignore globs to the PATTERNS array. */ > - SVN_ERR(svn_wc__db_read_props(&props, db, local_abspath, > - scratch_pool, scratch_pool)); > + if (err->apr_err != SVN_ERR_WC_PATH_UNEXPECTED_STATUS) > + return svn_error_trace(err); > > - if (!props) > - return SVN_NO_ERROR; > + svn_error_clear(err); > + return SVN_NO_ERROR; > + } > + > + if (props) > + { > + const svn_string_t *value; > > - value = apr_hash_get(props, SVN_PROP_IGNORE, APR_HASH_KEY_STRING); > + value = svn_hash_gets(props, SVN_PROP_IGNORE); > + if (value) > + svn_cstring_split_append(*patterns, value->data, "\n\r", FALSE, > + result_pool); > > - if (value != NULL) > + value = svn_hash_gets(props, SVN_PROP_INHERITABLE_IGNORES); > + if (value) > svn_cstring_split_append(*patterns, value->data, "\n\r", FALSE, > result_pool); > } > > - if (inherited_patterns) > + for (i = 0; i < inherited_props->nelts; i++) > { > - apr_array_header_t *inherited_props; > + svn_prop_inherited_item_t *elt = APR_ARRAY_IDX( > + inherited_props, i, svn_prop_inherited_item_t *); > + const svn_string_t *value; > > - *inherited_patterns = apr_array_make(result_pool, 1, > - sizeof(const char *)); > - if (props) > - { > - value = apr_hash_get(props, SVN_PROP_INHERITABLE_IGNORES, > - APR_HASH_KEY_STRING); > - if (value != NULL) > - svn_cstring_split_append(*inherited_patterns, value->data, > "\n\r", > - FALSE, result_pool); > - } > - > - SVN_ERR(svn_wc__db_read_inherited_props(&inherited_props, > - db, local_abspath, > - SVN_PROP_INHERITABLE_IGNORES, > - scratch_pool, scratch_pool)); > - for (i = 0; i < inherited_props->nelts; i++) > - { > - apr_hash_index_t *hi; > - svn_prop_inherited_item_t *elt = APR_ARRAY_IDX( > - inherited_props, i, svn_prop_inherited_item_t *); > - > - for (hi = apr_hash_first(scratch_pool, elt->prop_hash); > - hi; > - hi = apr_hash_next(hi)) > - { > - const svn_string_t *propval = svn__apr_hash_index_val(hi); > - svn_cstring_split_append(*inherited_patterns, propval->data, > - "\n\r", FALSE, result_pool); > - } > - } > + value = svn_hash_gets(elt->prop_hash, SVN_PROP_INHERITABLE_IGNORES); > + > + if (value) > + svn_cstring_split_append(*patterns, value->data, > + "\n\r", FALSE, result_pool); > } > > return SVN_NO_ERROR; > @@ -1097,26 +1086,21 @@ send_unversioned_item(const struct walk_ > const svn_io_dirent2_t *dirent, > svn_boolean_t tree_conflicted, > const apr_array_header_t *patterns, > - const apr_array_header_t *inherited_patterns, > svn_boolean_t no_ignore, > svn_wc_status_func4_t status_func, > void *status_baton, > apr_pool_t *scratch_pool) > { > svn_boolean_t is_ignored; > - svn_boolean_t is_mandatory_ignored; > svn_boolean_t is_external; > svn_wc_status3_t *status; > const char *base_name = svn_dirent_basename(local_abspath, NULL); > > is_ignored = svn_wc_match_ignore_list(base_name, patterns, scratch_pool); > - is_mandatory_ignored = svn_wc_match_ignore_list(base_name, > - inherited_patterns, > - scratch_pool); > SVN_ERR(assemble_unversioned(&status, > wb->db, local_abspath, > dirent, tree_conflicted, > - is_ignored || is_mandatory_ignored, > + is_ignored, > scratch_pool, scratch_pool)); > > is_external = is_external_path(wb->externals, local_abspath, scratch_pool); > @@ -1132,7 +1116,7 @@ send_unversioned_item(const struct walk_ > /* If we aren't ignoring it, or if it's an externals path, pass this > entry to the status func. */ > if (no_ignore > - || !(is_ignored || is_mandatory_ignored) > + || !is_ignored > || is_external) > return svn_error_trace((*status_func)(status_baton, local_abspath, > status, scratch_pool)); > @@ -1181,20 +1165,18 @@ get_dir_status(const struct walk_status_ > * > * DIR_HAS_PROPS is a boolean indicating whether PARENT_ABSPATH has > properties. > * > - * If *COLLECTED_IGNORE_PATTERNS or COLLECTED_INHERITED_IGNORE_PATTERNS are > NULL > - * and ignore patterns are needed in this call, then > *COLLECTED_IGNORE_PATTERNS > - * *COLLECTED_INHERITED_IGNORE_PATTERNS will be set to an apr_array_header_t* > + * If *COLLECTED_IGNORE_PATTERNS is NULL and ignore patterns are needed in > this > + * call, then *COLLECTED_IGNORE_PATTERNS will be set to an > apr_array_header_t* > * containing all ignore patterns, as returned by collect_ignore_patterns() > on > - * PARENT_ABSPATH and IGNORE_PATTERNS. If *COLLECTED_IGNORE_PATTERNS and > - * COLLECTED_INHERITED_IGNORE_PATTERNS is passed non-NULL, it is assumed they > - * already hold those results. This speeds up repeated calls with the same > - * PARENT_ABSPATH. > + * PARENT_ABSPATH and IGNORE_PATTERNS. If *COLLECTED_IGNORE_PATTERNS is > passed > + * non-NULL, it is assumed it already holds those results. > + * This speeds up repeated calls with the same PARENT_ABSPATH. > * > - * *COLLECTED_IGNORE_PATTERNS and COLLECTED_INHERITED_IGNORE_PATTERNS will be > - * allocated in RESULT_POOL. All other allocations are made in SCRATCH_POOL. > + * *COLLECTED_IGNORE_PATTERNS will be allocated in RESULT_POOL. All other > + * allocations are made in SCRATCH_POOL. > * > * The remaining parameters correspond to get_dir_status(). */ > -static svn_error_t* > +static svn_error_t * > one_child_status(const struct walk_status_baton *wb, > const char *local_abspath, > const char *parent_abspath, > @@ -1203,10 +1185,8 @@ one_child_status(const struct walk_statu > const char *dir_repos_root_url, > const char *dir_repos_relpath, > const char *dir_repos_uuid, > - svn_boolean_t dir_has_props, > svn_boolean_t unversioned_tree_conflicted, > apr_array_header_t **collected_ignore_patterns, > - apr_array_header_t **collected_inherited_ignore_patterns, > const apr_array_header_t *ignore_patterns, > svn_depth_t depth, > svn_boolean_t get_all, > @@ -1289,13 +1269,9 @@ one_child_status(const struct walk_statu > * determined. For example, in 'svn status', plain unversioned nodes show > * as '? C', where ignored ones show as 'I C'. */ > > - if ((ignore_patterns && ! *collected_ignore_patterns) > - || (collected_inherited_ignore_patterns > - && ! collected_inherited_ignore_patterns)) > + if (ignore_patterns && ! *collected_ignore_patterns) > SVN_ERR(collect_ignore_patterns(collected_ignore_patterns, > - collected_inherited_ignore_patterns, > wb->db, parent_abspath, ignore_patterns, > - dir_has_props, > result_pool, scratch_pool)); > > SVN_ERR(send_unversioned_item(wb, > @@ -1303,7 +1279,6 @@ one_child_status(const struct walk_statu > dirent, > conflicted, > *collected_ignore_patterns, > - *collected_inherited_ignore_patterns, > no_ignore, > status_func, status_baton, > scratch_pool)); > @@ -1357,7 +1332,6 @@ get_dir_status(const struct walk_status_ > apr_hash_t *dirents, *nodes, *conflicts, *all_children; > apr_array_header_t *sorted_children; > apr_array_header_t *collected_ignore_patterns = NULL; > - apr_array_header_t *collected_inherited_ignore_patterns = NULL; > apr_pool_t *iterpool; > svn_error_t *err; > int i; > @@ -1486,10 +1460,8 @@ get_dir_status(const struct walk_status_ > dir_repos_root_url, > dir_repos_relpath, > dir_repos_uuid, > - dir_has_props, > apr_hash_get(conflicts, key, klen) != NULL, > &collected_ignore_patterns, > - &collected_inherited_ignore_patterns, > ignore_patterns, > depth, > get_all, > @@ -1539,7 +1511,6 @@ get_child_status(const struct walk_statu > const char *dir_repos_uuid; > const struct svn_wc__db_info_t *dir_info; > apr_array_header_t *collected_ignore_patterns = NULL; > - apr_array_header_t *collected_inherited_ignore_patterns = NULL; > const char *parent_abspath = svn_dirent_dirname(local_abspath, > scratch_pool); > > @@ -1571,10 +1542,8 @@ get_child_status(const struct walk_statu > dir_repos_root_url, > dir_repos_relpath, > dir_repos_uuid, > - (dir_info->had_props || dir_info->props_mod), > FALSE, /* unversioned_tree_conflicted */ > &collected_ignore_patterns, > - &collected_inherited_ignore_patterns, > ignore_patterns, > svn_depth_empty, > get_all, > @@ -3092,8 +3061,8 @@ svn_wc_get_ignores2(apr_array_header_t * > apr_array_header_t *default_ignores; > > SVN_ERR(svn_wc_get_default_ignores(&default_ignores, config, > scratch_pool)); > - return svn_error_trace(collect_ignore_patterns(patterns, NULL, wc_ctx->db, > + return svn_error_trace(collect_ignore_patterns(patterns, wc_ctx->db, > local_abspath, > - default_ignores, TRUE, > + default_ignores, > result_pool, scratch_pool)); > } > > Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1453925&r1=1453924&r2=1453925&view=diff > ============================================================================== > --- subversion/trunk/subversion/libsvn_wc/wc_db.c (original) > +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Thu Mar 7 15:46:08 2013 > @@ -9641,7 +9641,8 @@ filter_unwanted_props(apr_hash_t *prop_h > /* The body of svn_wc__db_read_inherited_props(). > */ > static svn_error_t * > -db_read_inherited_props(apr_array_header_t **iprops, > +db_read_inherited_props(const apr_array_header_t **inherited_props, > + apr_hash_t **actual_props, > svn_wc__db_wcroot_t *wcroot, > const char *local_relpath, > const char *propname, > @@ -9650,54 +9651,112 @@ db_read_inherited_props(apr_array_header > { > int i; > apr_array_header_t *cached_iprops = NULL; > - const char *parent_relpath = local_relpath; > - svn_boolean_t is_wc_root = FALSE; > + apr_array_header_t *iprops; > apr_pool_t *iterpool = svn_pool_create(scratch_pool); > + svn_sqlite__stmt_t *stmt; > + apr_hash_t *props = NULL; > + const char *relpath; > + const char *expected_parent_repos_relpath = NULL; > + const char *parent_relpath; > > - *iprops = apr_array_make(result_pool, 1, > + iprops = apr_array_make(result_pool, 1, > sizeof(svn_prop_inherited_item_t *)); > + *inherited_props = iprops; > + > + SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb, > + STMT_SELECT_NODE_INFO)); > + > + relpath = local_relpath; > > /* Walk up to the root of the WC looking for inherited properties. When we > reach the WC root also check for cached inherited properties. */ > - while (TRUE) > + for (relpath = local_relpath; relpath; relpath = parent_relpath) > { > apr_hash_t *actual_props; > - svn_boolean_t is_switched; > + svn_boolean_t have_row; > + int op_depth; > + svn_wc__db_status_t status; > + > + parent_relpath = relpath[0] ? svn_relpath_dirname(relpath, > scratch_pool) > + : NULL; > > svn_pool_clear(iterpool); > > - if (*parent_relpath == '\0') > + SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, relpath)); > + > + SVN_ERR(svn_sqlite__step(&have_row, stmt)); > + > + if (!have_row) > + return svn_error_createf( > + SVN_ERR_WC_PATH_NOT_FOUND, svn_sqlite__reset(stmt), > + _("The node '%s' was not found."), > + path_for_error_message(wcroot, relpath, > + scratch_pool)); > + > + op_depth = svn_sqlite__column_int(stmt, 0); > + > + status = svn_sqlite__column_token(stmt, 3, presence_map); > + > + if (status != svn_wc__db_status_normal > + && status != svn_wc__db_status_incomplete) > + return svn_error_createf( > + SVN_ERR_WC_PATH_UNEXPECTED_STATUS, > svn_sqlite__reset(stmt), > + _("The node '%s' has a status that has no properties."), > + path_for_error_message(wcroot, relpath, > + scratch_pool)); > + > + if (op_depth > 0) > { > - is_switched = FALSE; > - is_wc_root = TRUE; > + /* WORKING node. Nothing to check */ > } > - else > - SVN_ERR(db_is_switched(&is_switched, NULL, wcroot, parent_relpath, > - scratch_pool)); > + else if (expected_parent_repos_relpath) > + { > + const char *repos_relpath = svn_sqlite__column_text(stmt, 2, NULL); > > - if (is_switched || is_wc_root) > + if (strcmp(expected_parent_repos_relpath, repos_relpath) != 0) > + { > + /* The child of this node has a different parent than this node > + (It is "switched"), so we can stop here. Note that switched > + with the same parent is not interesting for us here. */ > + SVN_ERR(svn_sqlite__reset(stmt)); > + break; > + } > + > + expected_parent_repos_relpath = > + svn_relpath_dirname(expected_parent_repos_relpath, > scratch_pool); > + } > + else > { > - is_wc_root = TRUE; > + const char *repos_relpath = svn_sqlite__column_text(stmt, 2, NULL); > > - /* If the WC root is also the root of the repository then by > - definition there are no inheritable properties to be had, > - but checking for that is just as expensive as fetching them > - anyway. */ > + expected_parent_repos_relpath = > + svn_relpath_dirname(repos_relpath, scratch_pool); > + } > > - /* Grab the cached inherited properties for the WC root. */ > - SVN_ERR(db_read_cached_iprops(&cached_iprops, > - wcroot, parent_relpath, > - result_pool, iterpool)); > + if (op_depth == 0 > + && !svn_sqlite__column_is_null(stmt, 16)) > + { > + /* The node contains a cache. No reason to look further */ > + SVN_ERR(svn_sqlite__column_iprops(&cached_iprops, stmt, 16, > + result_pool, iterpool)); > + > + parent_relpath = NULL; /* Stop after this */ > } > > - /* If PARENT_ABSPATH is a true parent of LOCAL_ABSPATH, then > - LOCAL_ABSPATH can inherit properties from it. */ > - if (strcmp(local_relpath, parent_relpath) != 0) > + if (relpath == local_relpath && actual_props) > + SVN_ERR(svn_sqlite__column_properties(&props, stmt, 14, > + result_pool, scratch_pool)); > + > + SVN_ERR(svn_sqlite__reset(stmt)); > + > + /* If PARENT_ABSPATH is a parent of LOCAL_ABSPATH, then LOCAL_ABSPATH > + can inherit properties from it. */ > + if (relpath != local_relpath) > { > - SVN_ERR(db_read_props(&actual_props, wcroot, parent_relpath, > + SVN_ERR(db_read_props(&actual_props, wcroot, relpath, > result_pool, iterpool)); > > - if (actual_props) > + if (actual_props && apr_hash_count(actual_props)) > { > /* If we only want PROPNAME filter out any other properties. */ > if (propname) > @@ -9709,22 +9768,15 @@ db_read_inherited_props(apr_array_header > apr_pcalloc(result_pool, > sizeof(svn_prop_inherited_item_t)); > iprop_elt->path_or_url = svn_dirent_join(wcroot->abspath, > - parent_relpath, > + relpath, > result_pool); > > iprop_elt->prop_hash = actual_props; > /* Build the output array in depth-first order. */ > - svn_sort__array_insert(&iprop_elt, *iprops, 0); > + svn_sort__array_insert(&iprop_elt, iprops, 0); > } > } > } > - > - /* Inheritance only goes as far as the nearest WC root. */ > - if (is_wc_root) > - break; > - > - /* Keep looking for the WC root. */ > - parent_relpath = svn_relpath_dirname(parent_relpath, scratch_pool); > } > > if (cached_iprops) > @@ -9745,16 +9797,34 @@ db_read_inherited_props(apr_array_header > > /* If we didn't filter everything then keep this iprop. */ > if (apr_hash_count(cached_iprop->prop_hash)) > - svn_sort__array_insert(&cached_iprop, *iprops, 0); > + svn_sort__array_insert(&cached_iprop, iprops, 0); > } > } > > + if (actual_props) > + { > + svn_boolean_t have_row; > + SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb, > + STMT_SELECT_ACTUAL_PROPS)); > + SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id, local_relpath)); > + SVN_ERR(svn_sqlite__step(&have_row, stmt)); > + > + if (have_row && !svn_sqlite__column_is_null(stmt, 0)) > + SVN_ERR(svn_sqlite__column_properties(actual_props, stmt, 0, > + result_pool, iterpool)); > + else > + *actual_props = props; /* Cached when we read that record */ > + > + SVN_ERR(svn_sqlite__reset(stmt)); > + } > + > svn_pool_destroy(iterpool); > return SVN_NO_ERROR; > } > > svn_error_t * > -svn_wc__db_read_inherited_props(apr_array_header_t **iprops, > +svn_wc__db_read_inherited_props(const apr_array_header_t **iprops, > + apr_hash_t **actual_props, > svn_wc__db_t *db, > const char *local_abspath, > const char *propname, > @@ -9771,7 +9841,7 @@ svn_wc__db_read_inherited_props(apr_arra > scratch_pool, scratch_pool)); > VERIFY_USABLE_WCROOT(wcroot); > > - SVN_WC__DB_WITH_TXN(db_read_inherited_props(iprops, > + SVN_WC__DB_WITH_TXN(db_read_inherited_props(iprops, actual_props, > wcroot, local_relpath, > propname, > result_pool, scratch_pool), > wcroot); > > Modified: subversion/trunk/subversion/libsvn_wc/wc_db.h > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.h?rev=1453925&r1=1453924&r2=1453925&view=diff > ============================================================================== > --- subversion/trunk/subversion/libsvn_wc/wc_db.h (original) > +++ subversion/trunk/subversion/libsvn_wc/wc_db.h Thu Mar 7 15:46:08 2013 > @@ -2120,11 +2120,15 @@ svn_wc__db_read_pristine_props(apr_hash_ > * paths relative to the repository root URL for cached inherited > * properties and absolute working copy paths otherwise. > * > + * If ACTUAL_PROPS is not NULL, then set *ACTUAL_PROPS to the actual > + * properties stored on LOCAL_ABSPATH. > + * > * Allocate @a *iprops in @a result_pool. Use @a scratch_pool > * for temporary allocations. > */ > svn_error_t * > -svn_wc__db_read_inherited_props(apr_array_header_t **iprops, > +svn_wc__db_read_inherited_props(const apr_array_header_t **iprops, > + apr_hash_t **actual_props, > svn_wc__db_t *db, > const char *local_abspath, > const char *propname, > >
-- Paul T. Burba CollabNet, Inc. -- www.collab.net -- Enterprise Cloud Development Skype: ptburba
