Hi Brane,
I don’t think the BASE optimization into svn_wc__db_read_children_info() is required to obtain good performance. In most working copies (I would hope :)) most nodes are status normal so the normal call will obtain the information you need. And as we don’t report symlink vs file in the status output (and no last_changed_* for deleted) there would just be an additional svn_wc__db_base_get_info() on replacements.
(Which the ‘svn’ client already has… but that is a different story)
Otherwise +1 on using a different query with ‘AND op_depth=0’.
(Adding a Boolean as flag to the query breaks the sqlite optimizer for the query as it only performs optimizing once, before it knows the actual values passed)
The only reason we obtain the special flag is to show if a file is obstructing a symlink or vice versa, and in your case we don’t want to report that, so setting special to FALSE when handling things from status should ‘just work’. (If you implement this in wc_db.c I would say that we should obtain the proper value)
The remaining interesting case would be when we see obstructing working copies… as in that case we don’t support reading the nodes (and their children) by their abspaths.
Personally I wouldn’t be surprised if you could obtain the required performance for this feature by just skipping all file comparisions (and the individual filestats where necessary), instead of implementing the whole ignore everything local work.
I don’t think reading the directory entries per directory is that expensive…
Subversion <= 1.6 read them per node during status processing… That was really expensive on Windows. But it could be that reading all the directory details is expensive on !Windows now, but perhaps it might help to pass TRUE for only_check_type on svn_io_get_dirents3().
(On Windows that doesn’t change the performance, but I think it does on other platforms)
Bert
From: Branko Čibej [mailto:brane_at_wandisco.com]
Sent: dinsdag 1 april 2014 13:10
To: dev_at_subversion.apache.org
Subject: Re: svn commit: r1583599 - in /subversion/branches/remote-only-status/subversion: include/svn_client.h libsvn_wc/status.c tests/libsvn_client/client-test.c
On 01.04.2014 12:54, Bert Huijben wrote:
-----Original Message-----
From: brane_at_apache.org <mailto:brane_at_apache.org> [mailto:brane_at_apache.org]
Sent: dinsdag 1 april 2014 12:41
To: commits_at_subversion.apache.org <mailto:commits_at_subversion.apache.org>
Subject: svn commit: r1583599 - in /subversion/branches/remote-only-
status/subversion: include/svn_client.h libsvn_wc/status.c
tests/libsvn_client/client-test.c
Author: brane
Date: Tue Apr 1 10:41:29 2014
New Revision: 1583599
URL: http://svn.apache.org/r1583599
Log:
On the remote-only-status branch: Do not report local replacements.
* subversion/include/svn_client.h (svn_client_status6): Update docstring.
* subversion/libsvn_wc/status.c (assemble_status): Ignore local adds.
Report local replacements as deletions of the working node.
(get_dir_status): Remove redundant (and incorrect) filter for additions.
* subversion/tests/libsvn_client/client-test.c (test_remote_only_status):
Extend test case with an example of a local replacement.
Modified:
subversion/branches/remote-only-status/subversion/include/svn_client.h
subversion/branches/remote-only-status/subversion/libsvn_wc/status.c
subversion/branches/remote-only-
status/subversion/tests/libsvn_client/client-test.c
Modified: subversion/branches/remote-only-
status/subversion/include/svn_client.h
URL: http://svn.apache.org/viewvc/subversion/branches/remote-only-
status/subversion/include/svn_client.h?rev=1583599&r1=1583598&r2=15835
99&view=diff
==========================================================
====================
--- subversion/branches/remote-only-
status/subversion/include/svn_client.h (original)
+++ subversion/branches/remote-only-
status/subversion/include/svn_client.h Tue Apr 1 10:41:29 2014
@@ -2512,6 +2512,13 @@ typedef svn_error_t *(*svn_client_status
* - If @a check_working_copy is not set, do not scan the working
* copy for locally modified and missing files. This parameter
* will be ignored unless @a check_out_of_date is set.
+ * When set, the status report will be different in the following
+ * details:
+ *
+ * -- Local modifications, missing nodes and locally added nodes
+ * will not be reported.
+ * -- Locally replaced nodes will be reported as deletions of
+ * the original node instead of as replacements.
*
* If @a no_ignore is @c FALSE, don't report any file or directory (or
* recurse into any directory) that is found by recursion (as opposed to
Modified: subversion/branches/remote-only-
status/subversion/libsvn_wc/status.c
URL: http://svn.apache.org/viewvc/subversion/branches/remote-only-
status/subversion/libsvn_wc/status.c?rev=1583599&r1=1583598&r2=158359
9&view=diff
==========================================================
====================
--- subversion/branches/remote-only-status/subversion/libsvn_wc/status.c
(original)
+++ subversion/branches/remote-only-
status/subversion/libsvn_wc/status.c Tue Apr 1 10:41:29 2014
@@ -573,7 +573,42 @@ assemble_status(svn_wc_status3_t **statu
if (below_working != svn_wc__db_status_not_present
&& below_working != svn_wc__db_status_deleted)
{
- node_status = svn_wc_status_replaced;
+ if (check_working_copy)
+ node_status = svn_wc_status_replaced;
+ else
+ {
+ /* This is a remote-only walk; report the
+ base node info instead of the replacement. */
+ const char *target;
+ const svn_checksum_t *checksum;
+ struct svn_wc__db_info_t *base_info =
+ apr_palloc(scratch_pool, sizeof(*base_info));
+ memcpy(base_info, info, sizeof(*base_info));
+ SVN_ERR(svn_wc__db_read_pristine_info(
+ &base_info->status,
+ &base_info->kind,
+ &base_info->changed_rev,
+ &base_info->changed_date,
+ &base_info->changed_author,
+ &base_info->depth,
+ &checksum, &target,
+ &base_info->had_props, NULL,
+ db, local_abspath,
+ scratch_pool, scratch_pool));
+ SVN_ERR(svn_wc__db_base_get_info(
+ NULL, NULL, &base_info->revnum,
+ NULL, NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ NULL, NULL, NULL, NULL,
+ db, local_abspath,
+ scratch_pool, scratch_pool));
If you really want the repository information you should read all the values using svn_wc__db_base_get_info as the changed* values read by svn_wc__db_read_pristine_info are those of the highest layer...
Only in case of a BASE-delete (not in case of a working delete, or a replacement!) do they represent the information you want.
+ base_info->has_checksum = (checksum != NULL);
+#ifdef HAVE_SYMLINK
+ base_info->special = (target != NULL);
Target is not used (yet)... you must read the properties to determine if a node is a symlink or not. I think you can get the property values from svn_wc__db_base_get_info() now.
+#endif
Bert
+ node_status = svn_wc_status_deleted;
+ info = base_info;
+ }
}
else
node_status = svn_wc_status_added;
@@ -610,6 +645,16 @@ assemble_status(svn_wc_status3_t **statu
&& prop_status != svn_wc_status_none)
node_status = prop_status;
+
+ /* Ignore local additions in remote-only mode */
+ if (!check_working_copy
+ && node_status == svn_wc_status_added
+ && !moved_from_abspath)
+ {
+ *status = NULL;
+ return SVN_NO_ERROR;
+ }
I don't think this really checks what you want to check here... I would guess you want to check the have_base value (too?), as replaced nodes will also have an added status.
(And even with that you might still miss a few edge cases in case parent directories are replaced with files, or the other way around)
Bert
Thanks for the review, again!
I'm actually thinking that this is really a hack, and I should just modify svn_wc__db_read_single_info and svn_wc__db_read_children_info to be aware of the remote-only flag. That's where the BASE->WORKING->ACTUAL overlay happens, and what I'm doing here is basically just trying to reproduce part of the result, which seems like a waste of code. IIUC, if I get the modifications just right, the additions and replacements won't show up in the results anyway, and I'll be able to revert this latest commit, and not have a higher-level filter for added nodes.
-- Brane
--
Branko Čibej | Director of Subversion
WANdisco // Non-Stop Data
e. brane_at_wandisco.com <mailto:brane_at_wandisco.com>
Received on 2014-04-01 13:58:20 CEST