Hi Bert, you gave the hint about per-dir optimization, so if you can spare the time, could you look at r1158491 and see whether you have some more hints?
Thanks! ~Neels On 08/17/2011 04:45 AM, ne...@apache.org wrote: > Author: neels > Date: Wed Aug 17 02:45:42 2011 > New Revision: 1158491 > > URL: http://svn.apache.org/viewvc?rev=1158491&view=rev > Log: > Follow-up to r1157292. > Optimize obtaining moved-to and moved-from for status by querying per-dir > where possible (suggested by: rhujiben). The original patch is modified so > that the status structs for nodes return moved-* information *only* on the > roots of a move, and *not* on their moved-along children. (I had intended to > return these on all moved nodes, but that does not perform well at all.) > > * subversion/include/svn_client.h (svn_client_status_t), > * subversion/include/svn_wc.h (svn_wc_status3_t): > Remove MOVED_TO_OP_ROOT_ABSPATH. Tweak comments for the other two MOVED_* > members (identical changes to both of these status related structs). > > * subversion/libsvn_client/status.c > (svn_client_status_dup, svn_client__create_status): > Apply removal of MOVED_TO_OP_ROOT_ABSPATH. > > * subversion/libsvn_wc/status.c > (read_info): > Use suboptimal ways to get MOVED_TO_ABSPATH and MOVED_HERE flag, until > better ways have been charted (only done for non-recursive status). > (assemble_status): > Remove the expensive scan_* calls for the benefit of the information > available from the per-dir status query. Scan the full MOVED_FROM_ABSPATH > *only* when MOVED_HERE is TRUE on an op-root. > (svn_wc_dup_status3): > Apply removal of MOVED_TO_OP_ROOT_ABSPATH. > > * subversion/libsvn_wc/wc_db.c > (read_children_info): > Return MOVED_TO_ABSPATH and MOVED_HERE via the modified > STMT_SELECT_NODE_CHILDREN_INFO query (s.b.). > > * subversion/libsvn_wc/wc_db.h > (svn_wc__db_info_t): Add MOVED_TO_ABSPATH and MOVED_HERE. > > * subversion/libsvn_wc/wc-queries.sql > (STMT_SELECT_NODE_CHILDREN_INFO): > Also select columns MOVED_HERE and MOVED_TO. > > * subversion/svn/status.c > (print_status): > Simplify condition as that decision is now taken elsewhere. > > > Modified: > subversion/trunk/subversion/include/svn_client.h > subversion/trunk/subversion/include/svn_wc.h > subversion/trunk/subversion/libsvn_client/status.c > subversion/trunk/subversion/libsvn_wc/status.c > subversion/trunk/subversion/libsvn_wc/wc-queries.sql > subversion/trunk/subversion/libsvn_wc/wc_db.c > subversion/trunk/subversion/libsvn_wc/wc_db.h > subversion/trunk/subversion/svn/status.c > > Modified: subversion/trunk/subversion/include/svn_client.h > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_client.h?rev=1158491&r1=1158490&r2=1158491&view=diff > ============================================================================== > --- subversion/trunk/subversion/include/svn_client.h (original) > +++ subversion/trunk/subversion/include/svn_client.h Wed Aug 17 02:45:42 2011 > @@ -2207,18 +2207,38 @@ typedef struct svn_client_status_t > const void *backwards_compatibility_baton; > > /** Set to the local absolute path that this node was moved from, if this > - * file or directory has been moved here locally. */ > + * file or directory has been moved here locally and is the root of that > + * move. Otherwise set to NULL. > + * > + * This will be NULL for moved-here nodes that are just part of a subtree > + * that was moved along (and are not themselves a root of a different move > + * operation). > + * > + * @since New in 1.8. */ > const char *moved_from_abspath; > > /** Set to the local absolute path that this node was moved to, if this > file > - * or directory has been moved away locally. */ > + * or directory has been moved away locally and corresponds to the root > + * of the destination side of the move. Otherwise set to NULL. > + * > + * Note: Saying just "root" here could be misleading. For example: > + * svn mv A AA; > + * svn mv AA/B BB; > + * creates a situation where A/B is moved-to BB, but one could argue that > + * the move source's root actually was AA/B. Note that, as far as the > + * working copy is concerned, above case is exactly identical to: > + * svn mv A/B BB; > + * svn mv A AA; > + * In both situations, @a moved_to_abspath would be set for nodes A (moved > + * to AA) and A/B (moved to BB), only. > + * > + * This will be NULL for moved-away nodes that were just part of a subtree > + * that was moved along (and are not themselves a root of a different move > + * operation). > + * > + * @since New in 1.8. */ > const char *moved_to_abspath; > > - /* If this file or directory has been moved away locally, set this to the > - * local absolute path that was the root of the move-away, i.e. to the > - * op-root of the delete-half of the move operation. */ > - const char *moved_to_op_root_abspath; > - > /* NOTE! Please update svn_client_status_dup() when adding new fields > here. */ > } svn_client_status_t; > > > Modified: subversion/trunk/subversion/include/svn_wc.h > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/include/svn_wc.h?rev=1158491&r1=1158490&r2=1158491&view=diff > ============================================================================== > --- subversion/trunk/subversion/include/svn_wc.h (original) > +++ subversion/trunk/subversion/include/svn_wc.h Wed Aug 17 02:45:42 2011 > @@ -3618,18 +3618,38 @@ typedef struct svn_wc_status3_t > /** @} */ > > /** Set to the local absolute path that this node was moved from, if this > - * file or directory has been moved here locally. */ > + * file or directory has been moved here locally and is the root of that > + * move. Otherwise set to NULL. > + * > + * This will be NULL for moved-here nodes that are just part of a subtree > + * that was moved along (and are not themselves a root of a different move > + * operation). > + * > + * @since New in 1.8. */ > const char *moved_from_abspath; > > /** Set to the local absolute path that this node was moved to, if this > file > - * or directory has been moved away locally. */ > + * or directory has been moved away locally and corresponds to the root > + * of the destination side of the move. Otherwise set to NULL. > + * > + * Note: Saying just "root" here could be misleading. For example: > + * svn mv A AA; > + * svn mv AA/B BB; > + * creates a situation where A/B is moved-to BB, but one could argue that > + * the move source's root actually was AA/B. Note that, as far as the > + * working copy is concerned, above case is exactly identical to: > + * svn mv A/B BB; > + * svn mv A AA; > + * In both situations, @a moved_to_abspath would be set for nodes A (moved > + * to AA) and A/B (moved to BB), only. > + * > + * This will be NULL for moved-away nodes that were just part of a subtree > + * that was moved along (and are not themselves a root of a different move > + * operation). > + * > + * @since New in 1.8. */ > const char *moved_to_abspath; > > - /* If this file or directory has been moved away locally, set this to the > - * local absolute path that was the root of the move-away, i.e. to the > - * op-root of the delete-half of the move operation. */ > - const char *moved_to_op_root_abspath; > - > /* NOTE! Please update svn_wc_dup_status3() when adding new fields here. */ > } svn_wc_status3_t; > > > Modified: subversion/trunk/subversion/libsvn_client/status.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/status.c?rev=1158491&r1=1158490&r2=1158491&view=diff > ============================================================================== > --- subversion/trunk/subversion/libsvn_client/status.c (original) > +++ subversion/trunk/subversion/libsvn_client/status.c Wed Aug 17 02:45:42 > 2011 > @@ -570,10 +570,6 @@ svn_client_status_dup(const svn_client_s > if (status->moved_to_abspath) > st->moved_to_abspath = apr_pstrdup(result_pool, > status->moved_to_abspath); > > - if (status->moved_to_op_root_abspath) > - st->moved_to_op_root_abspath = > - apr_pstrdup(result_pool, status->moved_to_op_root_abspath); > - > return st; > } > > @@ -680,7 +676,6 @@ svn_client__create_status(svn_client_sta > > (*cst)->moved_from_abspath = status->moved_from_abspath; > (*cst)->moved_to_abspath = status->moved_to_abspath; > - (*cst)->moved_to_op_root_abspath = status->moved_to_op_root_abspath; > > return SVN_NO_ERROR; > } > > Modified: subversion/trunk/subversion/libsvn_wc/status.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/status.c?rev=1158491&r1=1158490&r2=1158491&view=diff > ============================================================================== > --- subversion/trunk/subversion/libsvn_wc/status.c (original) > +++ subversion/trunk/subversion/libsvn_wc/status.c Wed Aug 17 02:45:42 2011 > @@ -273,6 +273,68 @@ read_info(const struct svn_wc__db_info_t > &mtb->lock, NULL, NULL, > db, local_abspath, > result_pool, scratch_pool)); > + > + if (mtb->status == svn_wc__db_status_deleted) > + { > + const char *moved_to_abspath; > + const char *moved_to_op_root_abspath; > + > + /* NOTE: we can't use op-root-ness as a condition here since a base > + * node can be the root of a move and still not be an explicit > + * op-root (having a working node with op_depth == pathelements). > + * > + * Both these (almost identical) situations showcase this: > + * svn mv a/b bb > + * svn del a > + * and > + * svn mv a aa > + * svn mv aa/b bb > + * In both, 'bb' is moved from 'a/b', but 'a/b' has no op_depth>0 > + * node at all, as its parent 'a' is locally deleted. */ > + > + SVN_ERR(svn_wc__db_scan_deletion(NULL, > + &moved_to_abspath, > + NULL, > + &moved_to_op_root_abspath, > + db, local_abspath, > + scratch_pool, scratch_pool)); > + if (moved_to_abspath != NULL > + && moved_to_op_root_abspath != NULL > + && strcmp(moved_to_abspath, moved_to_op_root_abspath) == 0) > + { > + mtb->moved_to_abspath = apr_pstrdup(result_pool, > + moved_to_abspath); > + } > + /* ### ^^^ THIS SUCKS. For at least two reasons: > + * 1) We scan the node deletion and that's technically not > necessary. > + * We'd be fine to know if this is an actual root of a move. > + * 2) From the elaborately calculated results, we backwards-guess > + * whether this is a root. > + * It works ok, and this code only gets called when a node is an > + * explicit target of a 'status'. But it would be better to do this > + * differently. > + * We could return moved-to via svn_wc__db_base_get_info() (called > + * just above), but as moved-to is only intended to be returned for > + * roots of a move, that doesn't fit too well. */ > + } > + } > + > + /* ### svn_wc__db_read_info() could easily return the moved-here flag. But > + * for now... (The per-dir query for recursive status is far more optimal.) > + * Note that this actually scans around to get the full path, for a bool. > + * This bool then gets returned, later is evaluated, and if true leads to > + * the same paths being scanned again. We'd want to obtain this bool here > as > + * cheaply as svn_wc__db_read_children_info() does. */ > + if (mtb->status == svn_wc__db_status_added) > + { > + const char *moved_from_abspath = NULL; > + SVN_ERR(svn_wc__db_scan_addition(NULL, NULL, NULL, NULL, NULL, > + NULL, NULL, NULL, NULL, > + &moved_from_abspath, > + NULL, > + db, local_abspath, > + result_pool, scratch_pool)); > + mtb->moved_here = (moved_from_abspath != NULL); > } > > mtb->has_checksum = (checksum != NULL); > @@ -404,8 +466,6 @@ assemble_status(svn_wc_status3_t **statu > const char *repos_root_url; > const char *repos_uuid; > const char *moved_from_abspath = NULL; > - const char *moved_to_abspath = NULL; > - const char *moved_to_op_root_abspath = NULL; > svn_filesize_t filesize = (dirent && (dirent->kind == svn_node_file)) > ? dirent->filesize > : SVN_INVALID_FILESIZE; > @@ -608,26 +668,20 @@ assemble_status(svn_wc_status3_t **statu > else if (schedule == svn_wc_schedule_replace) > node_status = svn_wc_status_replaced; > } > + > + /* Get moved-from info (only for potential op-roots of a move). */ > + if (node_status == svn_wc_status_added > + && info->moved_here > + && info->op_root) > + SVN_ERR(svn_wc__db_scan_addition(NULL, NULL, NULL, NULL, NULL, > + NULL, NULL, NULL, NULL, > + &moved_from_abspath, > + NULL, > + db, local_abspath, > + result_pool, scratch_pool)); > } > } > > - /* Get moved-to info. */ > - if (info->status == svn_wc__db_status_deleted) > - SVN_ERR(svn_wc__db_scan_deletion(NULL, > - &moved_to_abspath, > - NULL, > - &moved_to_op_root_abspath, > - db, local_abspath, > - result_pool, scratch_pool)); > - > - /* Get moved-from info. */ > - if (info->status == svn_wc__db_status_added) > - SVN_ERR(svn_wc__db_scan_addition(NULL, NULL, NULL, NULL, NULL, > - NULL, NULL, NULL, NULL, > - &moved_from_abspath, > - NULL, > - db, local_abspath, > - result_pool, scratch_pool)); > > if (node_status == svn_wc_status_normal) > node_status = text_status; > @@ -722,8 +776,7 @@ assemble_status(svn_wc_status3_t **statu > stat->repos_uuid = repos_uuid; > > stat->moved_from_abspath = moved_from_abspath; > - stat->moved_to_abspath = moved_to_abspath; > - stat->moved_to_op_root_abspath = moved_to_op_root_abspath; > + stat->moved_to_abspath = info->moved_to_abspath; > > *status = stat; > > @@ -2624,10 +2677,6 @@ svn_wc_dup_status3(const svn_wc_status3_ > new_stat->moved_to_abspath > = apr_pstrdup(pool, orig_stat->moved_to_abspath); > > - if (orig_stat->moved_to_op_root_abspath) > - new_stat->moved_to_op_root_abspath > - = apr_pstrdup(pool, orig_stat->moved_to_op_root_abspath); > - > /* Return the new hotness. */ > return new_stat; > } > > Modified: subversion/trunk/subversion/libsvn_wc/wc-queries.sql > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc-queries.sql?rev=1158491&r1=1158490&r2=1158491&view=diff > ============================================================================== > --- subversion/trunk/subversion/libsvn_wc/wc-queries.sql (original) > +++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Wed Aug 17 02:45:42 > 2011 > @@ -119,7 +119,7 @@ WHERE wc_id = ?1 AND local_relpath = ?2 > SELECT op_depth, nodes.repos_id, nodes.repos_path, presence, kind, revision, > checksum, translated_size, changed_revision, changed_date, changed_author, > depth, symlink_target, last_mod_time, properties, lock_token, lock_owner, > - lock_comment, lock_date, local_relpath > + lock_comment, lock_date, local_relpath, moved_here, moved_to > FROM nodes > LEFT OUTER JOIN lock ON nodes.repos_id = lock.repos_id > AND nodes.repos_path = lock.repos_relpath > > Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1158491&r1=1158490&r2=1158491&view=diff > ============================================================================== > --- subversion/trunk/subversion/libsvn_wc/wc_db.c (original) > +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Wed Aug 17 02:45:42 2011 > @@ -7038,6 +7038,8 @@ read_children_info(void *baton, > > if (op_depth == 0) > { > + const char *moved_to_relpath; > + > child_item->info.have_base = TRUE; > > /* Get the lock info. The query only reports lock info in the row > at > @@ -7045,11 +7047,20 @@ read_children_info(void *baton, > if (op_depth == 0) > child_item->info.lock = lock_from_columns(stmt, 15, 16, 17, 18, > result_pool); > + > + /* Moved-to is only stored at op_depth 0. */ > + moved_to_relpath = svn_sqlite__column_text(stmt, 21, NULL); > + if (moved_to_relpath) > + child_item->info.moved_to_abspath = > + svn_dirent_join(wcroot->abspath, moved_to_relpath, > result_pool); > } > else > { > child_item->nr_layers++; > child_item->info.have_more_work = (child_item->nr_layers > 1); > + > + /* Moved-here can only exist at op_depth > 0. */ > + child_item->info.moved_here = svn_sqlite__column_boolean(stmt, 20); > } > > err = svn_sqlite__step(&have_row, stmt); > > Modified: subversion/trunk/subversion/libsvn_wc/wc_db.h > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.h?rev=1158491&r1=1158490&r2=1158491&view=diff > ============================================================================== > --- subversion/trunk/subversion/libsvn_wc/wc_db.h (original) > +++ subversion/trunk/subversion/libsvn_wc/wc_db.h Wed Aug 17 02:45:42 2011 > @@ -1842,6 +1842,9 @@ struct svn_wc__db_info_t { > > svn_boolean_t locked; /* WC directory lock */ > svn_wc__db_lock_t *lock; /* Repository file lock */ > + > + const char *moved_to_abspath; /* Only on op-roots. See svn_wc_status3_t. */ > + svn_boolean_t moved_here; /* On both op-roots and children. */ > }; > > /* Return in *NODES a hash mapping name->struct svn_wc__db_info_t for > > Modified: subversion/trunk/subversion/svn/status.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/status.c?rev=1158491&r1=1158490&r2=1158491&view=diff > ============================================================================== > --- subversion/trunk/subversion/svn/status.c (original) > +++ subversion/trunk/subversion/svn/status.c Wed Aug 17 02:45:42 2011 > @@ -272,6 +272,10 @@ print_status(const char *path, > (*prop_conflicts)++; > } > > + /* Note that moved-from and moved-to information is only available in > STATUS > + * for (op-)roots of a move. Those are exactly the nodes we want to show > + * move info for in 'svn status'. See also comments in svn_wc_status3_t. */ > + > if (status->moved_from_abspath) > { > const char *cwd; > @@ -285,13 +289,7 @@ print_status(const char *path, > (char *)NULL); > } > > - /* Only print an extra moved-to line for the op-root of a move-away. > - * As each and every child node of a deleted tree is printed in status > - * output, each of them would be "duplicated" with a moved-to line. */ > - if (status->moved_to_abspath > - && status->moved_to_op_root_abspath > - && 0 == strcmp(status->moved_to_op_root_abspath, > - status->moved_to_abspath)) > + if (status->moved_to_abspath) > { > const char *cwd; > const char *relpath; > >
signature.asc
Description: OpenPGP digital signature
