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)); > } > > >
