You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Erik Huelsmann <eh...@gmail.com> on 2006/09/10 18:32:44 UTC
[PATCH] Issue 2607: svn keeps rewriting .svn/entries for committed items once for each item
Well, the patch does not address all of the issue's performance
problems, but it does address the case 'slow initial import through
in-place-import' and the 'add lots of items in one directory' for
which a testcase has been added by malcolmlr.
The case NOT handled is where paths in the same directory are affected
but have their own invocation of svn_wc_process_committed4. Ie this
generally works when entire trees need updating.
Comments?
Log:
[[[
Part of issue 2607: .svn/entries rewritten on each committed item.
Make recursive processing of committed items efficient.
* subversion/libsvn_wc/adm_ops.c
(process_committed_leaf): New. Appends log instructions to
update the given leaf-path (non-directory) after being committed.
(svn_wc_process_committed4): Use 1 logfile for all items in the
current directory (instead of 1 log per target), running that log
when all work in the current directory is done.
]]]
Index: subversion/libsvn_wc/adm_ops.c
===================================================================
--- subversion/libsvn_wc/adm_ops.c (revision 21400)
+++ subversion/libsvn_wc/adm_ops.c (working copy)
@@ -309,27 +309,24 @@
}
-svn_error_t *
-svn_wc_process_committed4(const char *path,
- svn_wc_adm_access_t *adm_access,
- svn_boolean_t recurse,
- svn_revnum_t new_revnum,
- const char *rev_date,
- const char *rev_author,
- apr_array_header_t *wcprop_changes,
- svn_boolean_t remove_lock,
- svn_boolean_t remove_changelist,
- const unsigned char *digest,
- apr_pool_t *pool)
+static svn_error_t *
+process_committed_leaf(svn_stringbuf_t *logtags,
+ const char *path,
+ svn_wc_adm_access_t *adm_access,
+ svn_revnum_t new_revnum,
+ const char *rev_date,
+ const char *rev_author,
+ apr_array_header_t *wcprop_changes,
+ svn_boolean_t remove_lock,
+ svn_boolean_t remove_changelist,
+ const unsigned char *digest,
+ apr_pool_t *pool)
{
const char *base_name;
- svn_stringbuf_t *logtags;
const char *hex_digest = NULL;
svn_wc_entry_t tmp_entry;
apr_uint32_t modify_flags = 0;
- logtags = svn_stringbuf_create("", pool);
-
SVN_ERR(svn_wc__adm_write_check(adm_access));
/* Set PATH's working revision to NEW_REVNUM; if REV_DATE and
@@ -461,12 +458,38 @@
}
}
- /* Write our accumulation of log entries into a log file */
- SVN_ERR(svn_wc__write_log(adm_access, 0, logtags, pool));
+ return SVN_NO_ERROR;
+}
- /* Run the log file we just created. */
- SVN_ERR(svn_wc__run_log(adm_access, NULL, pool));
+svn_error_t *
+svn_wc_process_committed4(const char *path,
+ svn_wc_adm_access_t *adm_access,
+ svn_boolean_t recurse,
+ svn_revnum_t new_revnum,
+ const char *rev_date,
+ const char *rev_author,
+ apr_array_header_t *wcprop_changes,
+ svn_boolean_t remove_lock,
+ svn_boolean_t remove_changelist,
+ const unsigned char *digest,
+ apr_pool_t *pool)
+{
+ const char *base_name;
+ const char *hex_digest = NULL;
+ svn_wc_entry_t tmp_entry;
+ apr_uint32_t modify_flags = 0;
+ int log_number = 1;
+ svn_stringbuf_t *logtags;
+
+ logtags = svn_stringbuf_create("", pool);
+
+ SVN_ERR(process_committed_leaf(logtags, path, adm_access,
+ new_revnum, rev_date, rev_author,
+ wcprop_changes,
+ remove_lock, remove_changelist,
+ digest, pool));
+
if (recurse)
{
apr_hash_t *entries;
@@ -509,16 +532,28 @@
a directory. Pass null for wcprop_changes, because the
ones present in the current call are only applicable to
this one committed item. */
- SVN_ERR(svn_wc_process_committed4
- (this_path, child_access,
- (current_entry->kind == svn_node_dir) ? TRUE : FALSE,
- new_revnum, rev_date, rev_author, NULL, FALSE,
- remove_changelist, NULL, subpool));
+ if (current_entry->kind == svn_node_dir)
+ SVN_ERR(svn_wc_process_committed4
+ (this_path, child_access,
+ TRUE,
+ new_revnum, rev_date, rev_author, NULL, FALSE,
+ remove_changelist, NULL, subpool));
+ else
+ SVN_ERR(process_committed_leaf
+ (logtags, this_path, adm_access,
+ new_revnum, rev_date, rev_author, NULL, FALSE,
+ remove_changelist, NULL, subpool));
}
svn_pool_destroy(subpool);
}
+ /* Write our accumulation of log entries into a log file */
+ SVN_ERR(svn_wc__write_log(adm_access, log_number, logtags, pool));
+
+ /* Run the log file(s) we just created. */
+ SVN_ERR(svn_wc__run_log(adm_access, NULL, pool));
+
return SVN_NO_ERROR;
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue 2607: svn keeps rewriting .svn/entries for
committed items once for each item
Posted by Philip Martin <ph...@codematters.co.uk>.
"Erik Huelsmann" <eh...@gmail.com> writes:
> Given that I found 1-item logs to be between 190 and 230 bytes, a
> post-commit of 50.000 items would require a log file of 10 MB. That
> justifies the 1-logfile-per-target approach (since we can't append
> logfiles).
I think you have neglected the need to grow the stringbuf in the
absence of realloc for pool memory, so more like 20MB. That's still
not a huge amount, but it's increasing faster than linear.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue 2607: svn keeps rewriting .svn/entries for committed items once for each item
Posted by Erik Huelsmann <eh...@gmail.com>.
On 9/11/06, Philip Martin <ph...@codematters.co.uk> wrote:
> "Garrett Rooney" <ro...@electricjellyfish.net> writes:
>
> > On 9/10/06, Erik Huelsmann <eh...@gmail.com> wrote:
> >
> >> + /* Write our accumulation of log entries into a log file */
> >> + SVN_ERR(svn_wc__write_log(adm_access, log_number, logtags, pool));
> >
> > I honestly have no idea here, this isn't my area of expertise, but is
> > holding the log data in memory like that a good idea? Is it
> > potentially big? Should we be appending to a temporary log file
> > instead? Or is this just "the way it's done"?
>
> The wc library supports multiple log files: log, log.1, log.2, etc.
> and it was introduced to solve a similar entries rewriting problem
> during update. I don't know whether Eric intended to use multiple log
> file or not, his patch introduces the log_number variable but never
> increments it.
Yes, I did do it with intent, because performance degraded between the
original and the version which used several logs. The log_number
variable is a left over from that era (which I found and deleted
locally already).
I have a different problem now though: I'm not getting the same
measurements I got yesterday. So, I can't be sure either version is
faster :-(
Given that I found 1-item logs to be between 190 and 230 bytes, a
post-commit of 50.000 items would require a log file of 10 MB. That
justifies the 1-logfile-per-target approach (since we can't append
logfiles).
bye,
Erik.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue 2607: svn keeps rewriting .svn/entries for
committed items once for each item
Posted by Philip Martin <ph...@codematters.co.uk>.
"Garrett Rooney" <ro...@electricjellyfish.net> writes:
> On 9/10/06, Erik Huelsmann <eh...@gmail.com> wrote:
>
>> + /* Write our accumulation of log entries into a log file */
>> + SVN_ERR(svn_wc__write_log(adm_access, log_number, logtags, pool));
>
> I honestly have no idea here, this isn't my area of expertise, but is
> holding the log data in memory like that a good idea? Is it
> potentially big? Should we be appending to a temporary log file
> instead? Or is this just "the way it's done"?
The wc library supports multiple log files: log, log.1, log.2, etc.
and it was introduced to solve a similar entries rewriting problem
during update. I don't know whether Eric intended to use multiple log
file or not, his patch introduces the log_number variable but never
increments it.
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Re: [PATCH] Issue 2607: svn keeps rewriting .svn/entries for committed items once for each item
Posted by Garrett Rooney <ro...@electricjellyfish.net>.
On 9/10/06, Erik Huelsmann <eh...@gmail.com> wrote:
> + logtags = svn_stringbuf_create("", pool);
> +
> + SVN_ERR(process_committed_leaf(logtags, path, adm_access,
> + new_revnum, rev_date, rev_author,
> + wcprop_changes,
> + remove_lock, remove_changelist,
> + digest, pool));
> +
> if (recurse)
> {
> apr_hash_t *entries;
> @@ -509,16 +532,28 @@
> a directory. Pass null for wcprop_changes, because the
> ones present in the current call are only applicable to
> this one committed item. */
> - SVN_ERR(svn_wc_process_committed4
> - (this_path, child_access,
> - (current_entry->kind == svn_node_dir) ? TRUE : FALSE,
> - new_revnum, rev_date, rev_author, NULL, FALSE,
> - remove_changelist, NULL, subpool));
> + if (current_entry->kind == svn_node_dir)
> + SVN_ERR(svn_wc_process_committed4
> + (this_path, child_access,
> + TRUE,
> + new_revnum, rev_date, rev_author, NULL, FALSE,
> + remove_changelist, NULL, subpool));
> + else
> + SVN_ERR(process_committed_leaf
> + (logtags, this_path, adm_access,
> + new_revnum, rev_date, rev_author, NULL, FALSE,
> + remove_changelist, NULL, subpool));
> }
>
> svn_pool_destroy(subpool);
> }
>
> + /* Write our accumulation of log entries into a log file */
> + SVN_ERR(svn_wc__write_log(adm_access, log_number, logtags, pool));
I honestly have no idea here, this isn't my area of expertise, but is
holding the log data in memory like that a good idea? Is it
potentially big? Should we be appending to a temporary log file
instead? Or is this just "the way it's done"?
-garrett
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org