Re: svn commit: r39355 - trunk/subversion/libsvn_wc

On Sep 16, 2009, at 4:11 AM, Bert Huijben wrote:

> Author: rhuijben
> Date: Wed Sep 16 02:11:16 2009
> New Revision: 39355
>
> Log:
> Following up on r39338, r39340, add the option of flushing the entries
> cache to the remove entry and set depth database calls.
>
> Suggested by: gstein

While I agree that the entries need to be flushed in these cases
(since we are doing a write of the DB), I'm *very* leery about doing
it conditionally. It smacks of a premature optimization, and that's
the type of thing that we've spent *months* attempting to eliminate in
wc-1. Plus, doing this in only one function introduces an API
consistency across the wc_db APIs. I understand the desire to use the
cache, and not flush it prematurely, but in this case I don't think it
should be conditional.

-Hyrum

> * subversion/libsvn_wc/entries.c
> (svn_wc__set_depth): Update caller.
> (svn_wc__entry_remove): Update caller.
>
> * subversion/libsvn_wc/wc_db.c
> (includes): Remove svn_path.h
> (svn_wc__db_temp_op_remove_entry,
> svn_wc__db_temp_op_set_dir_depth): Add parameter for flushing
> entries
> and use navigate_to_parent to find the parent pdh.
>
> * subversion/libsvn_wc/wc_db.h
> (svn_wc__db_temp_op_remove_entry): Update prototype.
> (svn_wc__db_temp_op_set_dir_depth): Update prototype.
>
> Modified:
> trunk/subversion/libsvn_wc/entries.c
> trunk/subversion/libsvn_wc/wc_db.c
> trunk/subversion/libsvn_wc/wc_db.h
>
> Modified: trunk/subversion/libsvn_wc/entries.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/entries.c?pathrev=39355&r1=39354&r2=39355
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- trunk/subversion/libsvn_wc/entries.c Wed Sep 16 01:23:58 2009
> (r39354)
> +++ trunk/subversion/libsvn_wc/entries.c Wed Sep 16 02:11:16 2009
> (r39355)
> @@ -1581,6 +1581,7 @@ svn_wc__set_depth(svn_wc__db_t *db,
> return svn_error_return(svn_wc__db_temp_op_set_dir_depth(db,
>
> local_dir_abspath,
> depth,
> + FALSE,
>
> scratch_pool));
> }
>
> @@ -2608,6 +2609,7 @@ svn_wc__entry_remove(svn_wc__db_t *db,
>
> /* And then remove it from the database */
> return svn_error_return(svn_wc__db_temp_op_remove_entry(db,
> local_abspath,
> + FALSE,
>
> scratch_pool));
> }
>
>
> Modified: trunk/subversion/libsvn_wc/wc_db.c
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/wc_db.c?pathrev=39355&r1=39354&r2=39355
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- trunk/subversion/libsvn_wc/wc_db.c Wed Sep 16 01:23:58 2009
> (r39354)
> +++ trunk/subversion/libsvn_wc/wc_db.c Wed Sep 16 02:11:16 2009
> (r39355)
> @@ -28,7 +28,6 @@
> #include "svn_types.h"
> #include "svn_error.h"
> #include "svn_dirent_uri.h"
> -#include "svn_path.h"
> #include "svn_wc.h"
> #include "svn_checksum.h"
> #include "svn_pools.h"
> @@ -2747,6 +2746,7 @@ svn_wc__db_op_read_tree_conflict(svn_wc_
> svn_error_t *
> svn_wc__db_temp_op_remove_entry(svn_wc__db_t *db,
> const char *local_abspath,
> + svn_boolean_t flush_entry_cache,
> apr_pool_t *scratch_pool)
> {
> svn_wc__db_pdh_t *pdh;
> @@ -2762,20 +2762,20 @@ svn_wc__db_temp_op_remove_entry(svn_wc__
> scratch_pool, scratch_pool));
> VERIFY_USABLE_PDH(pdh);
>
> + if (flush_entry_cache)
> + flush_entries(pdh);
> +
> /* Check if we should remove it from the parent db instead */
> - if (svn_path_is_empty(current_relpath))
> + if (strcmp(current_relpath, "") == 0)
> {
> - SVN_ERR(parse_local_abspath(&pdh, &current_relpath, db,
> - svn_dirent_dirname(local_abspath,
> - scratch_pool),
> - svn_sqlite__mode_readwrite,
> - scratch_pool, scratch_pool));
> + SVN_ERR(navigate_to_parent(&pdh, pdh,
> svn_sqlite__mode_readwrite,
> + scratch_pool));
>
> VERIFY_USABLE_PDH(pdh);
> - current_relpath = svn_dirent_join(current_relpath,
> - svn_dirent_basename
> (local_abspath,
> - NULL),
> - scratch_pool);
> + current_relpath = svn_dirent_basename(local_abspath, NULL);
> +
> + if (flush_entry_cache)
> + flush_entries(pdh);
> }
>
> wcroot = pdh->wcroot;
> @@ -2800,6 +2800,7 @@ svn_error_t *
> svn_wc__db_temp_op_set_dir_depth(svn_wc__db_t *db,
> const char *local_abspath,
> svn_depth_t depth,
> + svn_boolean_t flush_entry_cache,
> apr_pool_t *scratch_pool)
> {
> svn_wc__db_pdh_t *pdh;
> @@ -2821,8 +2822,12 @@ svn_wc__db_temp_op_set_dir_depth(svn_wc_
> /* ### We set depth on working and base to match entry behavior.
> Maybe these should be separated later? */
>
> + if (flush_entry_cache)
> + flush_entries(pdh);
> +
> +
> /* ### setting depth exclude on a wcroot breaks svn_wc_crop() */
> - if (!svn_path_is_empty(current_relpath) || depth !=
> svn_depth_exclude)
> + if (strcmp(current_relpath, "") != 0 || depth != svn_depth_exclude)
> {
> SVN_ERR(svn_sqlite__get_statement(&stmt, sdb,
> STMT_UPDATE_BASE_DEPTH));
> SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id,
> current_relpath));
> @@ -2836,15 +2841,12 @@ svn_wc__db_temp_op_set_dir_depth(svn_wc_
> }
>
> /* Check if we should also set depth in the parent db */
> - if (svn_path_is_empty(current_relpath))
> + if (strcmp(current_relpath, "") == 0)
> {
> svn_error_t *err;
> -
> - err = parse_local_abspath(&pdh, &current_relpath, db,
> - svn_dirent_dirname(local_abspath,
> - scratch_pool),
> - svn_sqlite__mode_readwrite,
> - scratch_pool, scratch_pool);
> +
> + err = navigate_to_parent(&pdh, pdh, svn_sqlite__mode_readwrite,
> + scratch_pool);
>
> if (err && err->apr_err == SVN_ERR_WC_NOT_WORKING_COPY)
> {
> @@ -2855,16 +2857,16 @@ svn_wc__db_temp_op_set_dir_depth(svn_wc_
> else
> SVN_ERR(err);
>
> + if (flush_entry_cache)
> + flush_entries(pdh);
> +
> depth = (depth == svn_depth_exclude) ? svn_depth_exclude
> : svn_depth_infinity;
>
> VERIFY_USABLE_PDH(pdh);
> wcroot = pdh->wcroot;
> sdb = wcroot->sdb;
> - current_relpath = svn_dirent_join(current_relpath,
> - svn_dirent_basename
> (local_abspath,
> - NULL),
> - scratch_pool);
> + current_relpath = svn_dirent_basename(local_abspath, NULL);
>
> SVN_ERR(svn_sqlite__get_statement(&stmt, sdb,
> STMT_UPDATE_BASE_DEPTH));
> SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id,
> current_relpath));
> @@ -2875,7 +2877,6 @@ svn_wc__db_temp_op_set_dir_depth(svn_wc_
> SVN_ERR(svn_sqlite__bindf(stmt, "is", wcroot->wc_id,
> current_relpath));
> SVN_ERR(svn_sqlite__bind_text(stmt, 3, svn_depth_to_word
> (depth)));
> SVN_ERR(svn_sqlite__step_done(stmt));
> -
> }
>
> return SVN_NO_ERROR;
>
> Modified: trunk/subversion/libsvn_wc/wc_db.h
> URL: http://svn.collab.net/viewvc/svn/trunk/subversion/libsvn_wc/wc_db.h?pathrev=39355&r1=39354&r2=39355
> =
> =
> =
> =
> =
> =
> =
> =
> ======================================================================
> --- trunk/subversion/libsvn_wc/wc_db.h Wed Sep 16 01:23:58 2009
> (r39354)
> +++ trunk/subversion/libsvn_wc/wc_db.h Wed Sep 16 02:11:16 2009
> (r39355)
> @@ -1710,18 +1710,22 @@ svn_wc__db_temp_is_dir_deleted(svn_boole
> apr_pool_t *scratch_pool);
>
> /* Removes all references of LOCAL_ABSPATH from its working copy
> - using DB. */
> + using DB. When FLUSH_ENTRIES is set to TRUE, flush the related
> + entries caches. */
> svn_error_t *
> svn_wc__db_temp_op_remove_entry(svn_wc__db_t *db,
> const char *local_abspath,
> + svn_boolean_t flush_entry_cache,
> apr_pool_t *scratch_pool);
>
> /* Sets the depth of LOCAL_ABSPATH in its working copy to DEPTH
> - using DB */
> + using DB. When FLUSH_ENTRIES is set to TRUE, flush the related
> + entries caches. */
> svn_error_t *
> svn_wc__db_temp_op_set_dir_depth(svn_wc__db_t *db,
> const char *local_abspath,
> svn_depth_t depth,
> + svn_boolean_t flush_entry_cache,
> apr_pool_t *scratch_pool);
>
> ------------------------------------------------------
> http://subversion.tigris.org/ds/viewMessage.do?dsForumId=495&dsMessageId=2395419

------------------------------------------------------
http://subversion.tigris.org/ds/viewMessage.do?dsForumId=462&dsMessageId=2395587
Received on 2009-09-16 16:36:38 CEST