Hi Bert.
Any objection to me simplifying start_directory_update_txn(), as in the
attached patch? It appears that it's doing relatively a lot of work
every time just to decide whether the repos_relpath is going to be the
same as before, and thus decide whether to use a different update
statement that omits that parameter. It seems to that the work done,
and the code complexity, must far outweigh the cost of simply providing
the parameter every time. Am I missing something? I haven't profiled
it, as it looks like an obvious win.
I'll commit if no objection.
- Julian
Simplify a WC DB function. Remove a DB scan and a string compare, and
simply update the row in the same way every time.
* subversion/libsvn_wc/wc_db.c
(start_directory_update_txn): Simplify
* subversion/libsvn_wc/wc-queries.sql
(STMT_UPDATE_BASE_NODE_PRESENCE_AND_REVNUM): Remove.
--This line, and those below, will be ignored--
Index: subversion/libsvn_wc/wc-queries.sql
===================================================================
--- subversion/libsvn_wc/wc-queries.sql (revision 1022738)
+++ subversion/libsvn_wc/wc-queries.sql (working copy)
@@ -313,10 +313,6 @@ WHERE wc_id = ?1 AND local_relpath = ?2
AND op_depth = (SELECT MAX(op_depth) FROM nodes
WHERE wc_id = ?1 AND local_relpath = ?2 AND op_depth > 0);
--- STMT_UPDATE_BASE_NODE_PRESENCE_AND_REVNUM
-UPDATE nodes SET presence = ?3, revision = ?4
-WHERE wc_id = ?1 AND local_relpath = ?2 AND op_depth = 0;
-
-- STMT_UPDATE_BASE_NODE_PRESENCE_REVNUM_AND_REPOS_PATH
UPDATE nodes SET presence = ?3, revision = ?4, repos_path = ?5
WHERE wc_id = ?1 AND local_relpath = ?2 AND op_depth = 0;
--This line, and those below, will be ignored--
Index: subversion/libsvn_wc/wc_db.c
===================================================================
--- subversion/libsvn_wc/wc_db.c (revision 1022738)
+++ subversion/libsvn_wc/wc_db.c (working copy)
@@ -7500,42 +7500,22 @@ start_directory_update_txn(void *baton,
apr_pool_t *scratch_pool)
{
struct start_directory_update_baton *du = baton;
- const char *repos_relpath;
svn_sqlite__stmt_t *stmt;
- SVN_ERR(svn_wc__db_scan_base_repos(&repos_relpath, NULL, NULL,
- du->db, du->local_abspath,
- scratch_pool, scratch_pool));
-
- if (strcmp(du->new_repos_relpath, repos_relpath) == 0)
- {
- /* Just update revision and status */
- SVN_ERR(svn_sqlite__get_statement(
- &stmt, db,
- STMT_UPDATE_BASE_NODE_PRESENCE_AND_REVNUM));
-
- SVN_ERR(svn_sqlite__bindf(stmt, "isti",
- du->wc_id,
- du->local_relpath,
- presence_map, svn_wc__db_status_incomplete,
- (apr_int64_t)du->new_rev));
- SVN_ERR(svn_sqlite__step_done(stmt));
- }
- else
- {
- /* ### TODO: Maybe check if we can make repos_relpath NULL. */
- SVN_ERR(svn_sqlite__get_statement(
- &stmt, db,
- STMT_UPDATE_BASE_NODE_PRESENCE_REVNUM_AND_REPOS_PATH));
+ /* Note: In the majority of calls, the repos_relpath is unchanged. */
+ /* ### TODO: Maybe check if we can make repos_relpath NULL. */
+ SVN_ERR(svn_sqlite__get_statement(
+ &stmt, db,
+ STMT_UPDATE_BASE_NODE_PRESENCE_REVNUM_AND_REPOS_PATH));
+
+ SVN_ERR(svn_sqlite__bindf(stmt, "istis",
+ du->wc_id,
+ du->local_relpath,
+ presence_map, svn_wc__db_status_incomplete,
+ (apr_int64_t)du->new_rev,
+ du->new_repos_relpath));
+ SVN_ERR(svn_sqlite__step_done(stmt));
- SVN_ERR(svn_sqlite__bindf(stmt, "istis",
- du->wc_id,
- du->local_relpath,
- presence_map, svn_wc__db_status_incomplete,
- (apr_int64_t)du->new_rev,
- du->new_repos_relpath));
- SVN_ERR(svn_sqlite__step_done(stmt));
- }
return SVN_NO_ERROR;
}
