[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1346035 - in /subversion/trunk/subversion/libsvn_wc: questions.c wc-queries.sql wc_db.c wc_db.h workqueue.c

From: Greg Stein <gstein_at_gmail.com>
Date: Thu, 7 Jun 2012 15:58:18 -0400

Bert said he was planning to use a simple boolean to distinguish
between BASE and not-BASE.

Cheers,
-g

On Thu, Jun 7, 2012 at 1:04 PM, Julian Foad <julianfoad_at_btopenworld.com> wrote:
> 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 <gstein_at_gmail.com>
> To: dev_at_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, <rhuijben_at_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));
>  }
>
>
>
>
Received on 2012-06-07 21:58:51 CEST

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.