Note that op_depth (as a number) conveys the same information as an "op root" path. So maybe reworking this to pass an op-root path instead would be more acceptable?
- Julian >________________________________ > From: Greg Stein <gst...@gmail.com> >To: dev@subversion.apache.org >Sent: Monday, 4 June 2012, 20:01 >Subject: Re: svn commit: r1346035 - in /subversion/trunk/subversion/libsvn_wc: >questions.c wc-queries.sql wc_db.c wc_db.h workqueue.c > > >I don't like this at all. You're making the abstraction leaky by making >op_depth available. Users of wc_db should never know about the op_depth >concept. >Cheers, >-g >On Jun 4, 2012 1:05 PM, <rhuij...@apache.org> wrote: > >Author: rhuijben >>Date: Mon Jun 4 17:04:54 2012 >>New Revision: 1346035 >> >>URL: http://svn.apache.org/viewvc?rev=1346035&view=rev >>Log: >>Optimize the most common (update, checkout, commit) callers of >>svn_wc__db_global_record_fileinfo() to pass the op_depth we already >>retrieved in a previous file install specific wc_db call. >> >>This avoids a query (inside a lock) per affected file, during both update >>and commit processing. >> >>* subversion/libsvn_wc/questions.c >> (svn_wc__internal_file_modified_p): Pass -1 as we don't know the op_depth. >> >>* subversion/libsvn_wc/wc-queries.sql >> (STMT_UPDATE_NODE_FILEINFO_OPDEPTH): Add copy of STMT_UPDATE_NODE_FILEINFO >> with known op_depth. >> >>* subversion/libsvn_wc/wc_db.c >> (record_baton_t): Add op_depth. >> (db_record_fileinfo): Use more efficient statement if op_depth set. >> (svn_wc__db_global_record_fileinfo): Allow passing op_depth. Don't use >> lock as the call uses only one statement, so this only slows things down. >> (set_props_txn): Update caller to fill new field. >> >> (svn_wc__db_read_node_install_info): Add op_depth output argument. >> >>* subversion/libsvn_wc/wc_db.h >> (svn_wc__db_read_node_install_info, >> svn_wc__db_global_record_fileinfo): Update prototype and documentation. >> >>* subversion/libsvn_wc/workqueue.c >> (get_and_record_fileinfo): At op_depth argument. Update caller. >> (process_commit_file_install): Use the same code as other places. Pass 0 for >> op_depth. >> (run_file_install): Retrieve and pass op_depth. >> (run_record_fileinfo): Update caller for unknown op_depth. >> >>Modified: >> subversion/trunk/subversion/libsvn_wc/questions.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/libsvn_wc/workqueue.c >> >>Modified: subversion/trunk/subversion/libsvn_wc/questions.c >>URL: >>http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/questions.c?rev=1346035&r1=1346034&r2=1346035&view=diff >>============================================================================== >>--- subversion/trunk/subversion/libsvn_wc/questions.c (original) >>+++ subversion/trunk/subversion/libsvn_wc/questions.c Mon Jun 4 17:04:54 2012 >>@@ -360,7 +360,7 @@ svn_wc__internal_file_modified_p(svn_boo >> SVN_ERR(svn_wc__db_wclock_owns_lock(&own_lock, db, local_abspath, FALSE, >> scratch_pool)); >> if (own_lock) >>- SVN_ERR(svn_wc__db_global_record_fileinfo(db, local_abspath, >>+ SVN_ERR(svn_wc__db_global_record_fileinfo(db, local_abspath, -1, >> dirent->filesize, >> dirent->mtime, >> scratch_pool)); >> >>Modified: subversion/trunk/subversion/libsvn_wc/wc-queries.sql >>URL: >>http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc-queries.sql?rev=1346035&r1=1346034&r2=1346035&view=diff >>============================================================================== >>--- subversion/trunk/subversion/libsvn_wc/wc-queries.sql (original) >>+++ subversion/trunk/subversion/libsvn_wc/wc-queries.sql Mon Jun 4 17:04:54 >>2012 >>@@ -352,6 +352,10 @@ 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) >> >>+-- STMT_UPDATE_NODE_FILEINFO_OPDEPTH >>+UPDATE nodes SET translated_size = ?3, last_mod_time = ?4 >>+WHERE wc_id = ?1 AND local_relpath = ?2 AND op_depth = ?5 >>+ >> -- STMT_UPDATE_ACTUAL_TREE_CONFLICTS >> UPDATE actual_node SET tree_conflict_data = ?3 >> WHERE wc_id = ?1 AND local_relpath = ?2 >> >>Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c >>URL: >>http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1346035&r1=1346034&r2=1346035&view=diff >>============================================================================== >>--- subversion/trunk/subversion/libsvn_wc/wc_db.c (original) >>+++ subversion/trunk/subversion/libsvn_wc/wc_db.c Mon Jun 4 17:04:54 2012 >>@@ -4602,6 +4602,7 @@ svn_wc__db_op_add_symlink(svn_wc__db_t * >> struct record_baton_t { >> svn_filesize_t translated_size; >> apr_time_t last_mod_time; >>+ int op_depth; >> }; >> >> >>@@ -4617,9 +4618,15 @@ db_record_fileinfo(void *baton, >> int affected_rows; >> >> SVN_ERR(svn_sqlite__get_statement(&stmt, wcroot->sdb, >>- STMT_UPDATE_NODE_FILEINFO)); >>+ (rb->op_depth >= 0) >>+ ? STMT_UPDATE_NODE_FILEINFO_OPDEPTH >>+ : STMT_UPDATE_NODE_FILEINFO)); >> SVN_ERR(svn_sqlite__bindf(stmt, "isii", wcroot->wc_id, local_relpath, >> rb->translated_size, rb->last_mod_time)); >>+ >>+ if (rb->op_depth >= 0) >>+ SVN_ERR(svn_sqlite__bind_int(stmt, 5, rb->op_depth)); >>+ >> SVN_ERR(svn_sqlite__update(&affected_rows, stmt)); >> >> SVN_ERR_ASSERT(affected_rows == 1); >>@@ -4631,6 +4638,7 @@ db_record_fileinfo(void *baton, >> svn_error_t * >> svn_wc__db_global_record_fileinfo(svn_wc__db_t *db, >> const char *local_abspath, >>+ int op_depth, >> svn_filesize_t translated_size, >> apr_time_t last_mod_time, >> apr_pool_t *scratch_pool) >>@@ -4647,9 +4655,9 @@ svn_wc__db_global_record_fileinfo(svn_wc >> >> rb.translated_size = translated_size; >> rb.last_mod_time = last_mod_time; >>+ rb.op_depth = op_depth; >> >>- SVN_ERR(svn_wc__db_with_txn(wcroot, local_relpath, db_record_fileinfo, &rb, >>- scratch_pool)); >>+ SVN_ERR(db_record_fileinfo(&rb, wcroot, local_relpath, scratch_pool)); >> >> /* We *totally* monkeyed the entries. Toss 'em. */ >> SVN_ERR(flush_entries(wcroot, local_abspath, svn_depth_empty, >>scratch_pool)); >>@@ -4744,6 +4752,7 @@ set_props_txn(void *baton, >> struct record_baton_t rb; >> rb.translated_size = SVN_INVALID_FILESIZE; >> rb.last_mod_time = 0; >>+ rb.op_depth = -1; >> SVN_ERR(db_record_fileinfo(&rb, wcroot, local_relpath, scratch_pool)); >> } >> >>@@ -7939,6 +7948,7 @@ svn_wc__db_read_node_install_info(const >> const svn_checksum_t **sha1_checksum, >> apr_hash_t **pristine_props, >> apr_time_t *changed_date, >>+ int *op_depth, >> svn_wc__db_t *db, >> const char *local_abspath, >> const char *wri_abspath, >>@@ -7994,6 +8004,9 @@ svn_wc__db_read_node_install_info(const >> >> if (changed_date) >> *changed_date = svn_sqlite__column_int64(stmt, 9); >>+ >>+ if (op_depth) >>+ *op_depth = svn_sqlite__column_int(stmt, 0); >> } >> else >> return svn_error_createf(SVN_ERR_WC_PATH_NOT_FOUND, >> >>Modified: subversion/trunk/subversion/libsvn_wc/wc_db.h >>URL: >>http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.h?rev=1346035&r1=1346034&r2=1346035&view=diff >>============================================================================== >>--- subversion/trunk/subversion/libsvn_wc/wc_db.h (original) >>+++ subversion/trunk/subversion/libsvn_wc/wc_db.h Mon Jun 4 17:04:54 2012 >>@@ -1973,7 +1973,8 @@ svn_wc__db_read_pristine_info(svn_wc__db >> Set WCROOT_ABSPATH to the working copy root, SHA1_CHECKSUM to the >> checksum of the node (a valid reference into the pristine store) >> and PRISTINE_PROPS to the node's pristine properties (to use for >>- installing the file). >>+ installing the file). Set *CHANGED_DATE to the recorded changed date and >>+ *OP_DEPTH to the nodes highest/current op_depth. >> >> If WRI_ABSPATH is not NULL, check for information in the working copy >> identified by WRI_ABSPATH. >>@@ -1983,6 +1984,7 @@ svn_wc__db_read_node_install_info(const >> const svn_checksum_t **sha1_checksum, >> apr_hash_t **pristine_props, >> apr_time_t *changed_date, >>+ int *op_depth, >> svn_wc__db_t *db, >> const char *local_abspath, >> const char *wri_abspath, >>@@ -2400,9 +2402,8 @@ svn_wc__db_op_bump_revisions_post_update >> >> /* Record the TRANSLATED_SIZE and LAST_MOD_TIME for a versioned node. >> >>- This function will record the information within the WORKING node, >>- if present, or within the BASE tree. If neither node is present, then >>- SVN_ERR_WC_PATH_NOT_FOUND will be returned. >>+ This function will record the information within the highest WORKING node, >>+ or if OP_DEPTH >= 0 at the specified OP_DEPTH. >> >> TRANSLATED_SIZE may be SVN_INVALID_FILESIZE, which will be recorded >> as such, implying "unknown size". >>@@ -2413,6 +2414,7 @@ svn_wc__db_op_bump_revisions_post_update >> svn_error_t * >> svn_wc__db_global_record_fileinfo(svn_wc__db_t *db, >> const char *local_abspath, >>+ int op_depth, >> svn_filesize_t translated_size, >> apr_time_t last_mod_time, >> apr_pool_t *scratch_pool); >> >>Modified: subversion/trunk/subversion/libsvn_wc/workqueue.c >>URL: >>http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/workqueue.c?rev=1346035&r1=1346034&r2=1346035&view=diff >>============================================================================== >>--- subversion/trunk/subversion/libsvn_wc/workqueue.c (original) >>+++ subversion/trunk/subversion/libsvn_wc/workqueue.c Mon Jun 4 17:04:54 2012 >>@@ -72,6 +72,7 @@ struct work_item_dispatch { >> static svn_error_t * >> get_and_record_fileinfo(svn_wc__db_t *db, >> const char *local_abspath, >>+ int op_depth, >> svn_boolean_t ignore_enoent, >> apr_pool_t *scratch_pool) >> { >>@@ -87,7 +88,7 @@ get_and_record_fileinfo(svn_wc__db_t *db >> } >> >> return svn_error_trace(svn_wc__db_global_record_fileinfo( >>- db, local_abspath, >>+ db, local_abspath, op_depth, >> dirent->filesize, dirent->mtime, >> scratch_pool)); >> } >>@@ -463,13 +464,16 @@ process_commit_file_install(svn_wc__db_t >> /* We will compute and modify the size and timestamp */ >> if (overwrote_working) >> { >>- apr_finfo_t finfo; >>+ const svn_io_dirent2_t *dirent; >> >>- SVN_ERR(svn_io_stat(&finfo, local_abspath, >>- APR_FINFO_MIN | APR_FINFO_LINK, scratch_pool)); >>- SVN_ERR(svn_wc__db_global_record_fileinfo(db, local_abspath, >>- finfo.size, finfo.mtime, >>- scratch_pool)); >>+ SVN_ERR(svn_io_stat_dirent(&dirent, local_abspath, TRUE, >>+ scratch_pool, scratch_pool)); >>+ >>+ if (dirent->kind == svn_node_file) >>+ SVN_ERR(svn_wc__db_global_record_fileinfo(db, local_abspath, 0, >>+ dirent->filesize, >>+ dirent->mtime, >>+ scratch_pool)); >> } >> else >> { >>@@ -644,6 +648,7 @@ run_file_install(svn_wc__db_t *db, >> const svn_checksum_t *checksum; >> apr_hash_t *props; >> apr_time_t changed_date; >>+ int op_depth; >> >> local_relpath = apr_pstrmemdup(scratch_pool, arg1->data, arg1->len); >> SVN_ERR(svn_wc__db_from_relpath(&local_abspath, db, wri_abspath, >>@@ -656,7 +661,7 @@ run_file_install(svn_wc__db_t *db, >> >> SVN_ERR(svn_wc__db_read_node_install_info(&wcroot_abspath, >> &checksum, &props, >>- &changed_date, >>+ &changed_date, &op_depth, >> db, local_abspath, wri_abspath, >> scratch_pool, scratch_pool)); >> >>@@ -800,7 +805,7 @@ run_file_install(svn_wc__db_t *db, >> /* ### this should happen before we rename the file into place. */ >> if (record_fileinfo) >> { >>- SVN_ERR(get_and_record_fileinfo(db, local_abspath, >>+ SVN_ERR(get_and_record_fileinfo(db, local_abspath, op_depth, >> FALSE /* ignore_enoent */, >> scratch_pool)); >> } >>@@ -1244,7 +1249,7 @@ run_record_fileinfo(svn_wc__db_t *db, >> } >> >> >>- return svn_error_trace(get_and_record_fileinfo(db, local_abspath, >>+ return svn_error_trace(get_and_record_fileinfo(db, local_abspath, -1, >> TRUE /* ignore_enoent */, >> scratch_pool)); >> } >> >> >> > >
