Re: Problem with external file
Bob Fletcher robert.h.fletc...@ieee.org writes: To show the problem: (1) Create a project with an external file. (2) Modify and Commit a change to an actual working copy of the file (in another project). (3) Update the original project with the external file - even when omitting externals. This gives the following Subversion Exception: That's not enough detail to reproduce the problem. You need to describe every step, something like (I'm just guessing here): - create a repository - checkout a working copy - add a file wc/foo to the working copy - commit the file - add a file-external '^/foo bar' to the working copy root - commit the file-external - update the working copy - checkout a second working copy - modify the file in the second working copy - commit the second working copy - update the first working copy Ideally these would be commands we could execute using the command line client. -- Philip
Re: Format 20 upgrade to NODES
Philip Martin philip.mar...@wandisco.com writes: I'd like to enable NODES as a replacement for BASE_NODE and WORKING_NODE. This would involve bumping the format number, and old working copies would get automatically upgraded. This has been committed to trunk. -- Philip
Re: svn commit: r1003238 - in /subversion/trunk/subversion: svn/cl.h svn/propget-cmd.c svn/proplist-cmd.c svn/props.c tests/cmdline/prop_tests.py
Hi Paul. It's good to see a fix. I think, however, you could do this more simply and avoid some of the difficulties completely. On Thu, 2010-09-30, pbu...@apache.org wrote: Author: pburba Date: Thu Sep 30 20:21:19 2010 New Revision: 1003238 URL: http://svn.apache.org/viewvc?rev=1003238view=rev Log: Fix issue #3721 'redirection of svn propget output corrupted with large property values'. And this change also fixes an EOL-style inconsistency, which was presumably happening consistently with property values of any size, doesn't it? I think this change also introduces a new, similar, EOL-style inconsistency, in a different place; see last review comment at end of email. The difficulty with EOLs is that the native printf() family of functions typically converts to native EOLs, and the svn_stream_write() family doesn't, even when writing to svn_stream_for_stdout(). Therefore we have to be careful when mixing the two families. This change makes all the writes to stdout, by svn pg, via the svn_stream_t * that we already use to print the 'Properties on' header. Note that this stream *is* attached to stdout, but avoiding the mix of stream writes and printfs solves issue #3721 on Windows. It looks like the root of the corruption is also the mixing of native 'printf' functions with writes to a 'svn_stream_for_stdout()' stream. Do you agree that that's the case, and that therefore I should write a warning about this in the documentation of 'svn_stream_for_stdout()'? Perhaps not now, but we should consider whether we can eliminate the corruption by doing something to svn_stream_for_stdout() and/or the native 'stdout' stream. It might be possible by turning off some buffering that is happening, or something like that, or probably it could be done by implementing the stream_for_stdout as a stream class that forwards to the printf family of functions. What we probably should do now is avoid mixing printf with svn streams in this 'svn propget' code. From what I can see, there is little or no reason why svn_cl__propget() was using an svn stream for some of its printing at all. It looks like it would be much simpler to remove that stream (the variable called 'out') and just use printf-style functions throughout, as is done for 'proplist' and most of the rest of the UI (except for 'cat' and 'blame'). Note that we have svn_cmdline_printf() which is a handy encoding-converting variant of the native 'printf' family. Does that sound good? If so, then all the comments below become moot. (I wrote some of them before I figured all this out, and they may still be relevant if I'm wrong about some of this.) * subversion/svn/cl.h (svn_cl__print_prop_hash): Support printing to a passed in svn_stream_t *. * subversion/svn/propget-cmd.c (print_properties): Make the header's line endings native. Print the property names and values to the same svn_stream_t * we already use print the headers to. * subversion/svn/proplist-cmd.c (proplist_receiver, svn_cl__proplist): Update calls to svn_cl__print_prop_hash(), keeping old behavior. * subversion/svn/props.c (svn_cl__print_prop_hash): Support printing to a passed in svn_stream_t *, otherwise fall-back to old behavior of using printf(). * subversion/tests/cmdline/prop_tests.py (propget_redirection): Remove comment re failure status. (test_list): Remove XFail. Modified: subversion/trunk/subversion/svn/cl.h URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svn/cl.h?rev=1003238r1=1003237r2=1003238view=diff == --- subversion/trunk/subversion/svn/cl.h (original) +++ subversion/trunk/subversion/svn/cl.h Thu Sep 30 20:21:19 2010 @@ -417,15 +417,18 @@ svn_cl__print_status_xml(const char *pat apr_pool_t *pool); -/* Print a hash that maps property names (char *) to property values - (svn_string_t *). The names are assumed to be in UTF-8 format; +/* Print to stdout a hash that maps property names (char *) to property + values (svn_string_t *). The names are assumed to be in UTF-8 format; the values are either in UTF-8 (the special Subversion props) or plain binary values. + If OUT is not NULL, then write to it rather than stdout. This could do with pointing out that the two modes should not be mixed. This seems like unnecessary complexity, allowing the caller to choose whether to write to the native stdout stream or write to a supplied stream which in practice (at present) is always connected to stdout. Just support one or the other and make both callers consistent. + If NAMES_ONLY is true, print just names, else print names and values. */ svn_error_t * -svn_cl__print_prop_hash(apr_hash_t *prop_hash, +svn_cl__print_prop_hash(svn_stream_t *out, +apr_hash_t *prop_hash, svn_boolean_t
[PATCH] gpg-agent configure fix.
[[[ On the 'gpg-agent-password-store' branch: * configure.ac: (SVN_HAVE_GPG_AGENT): Defines when --with-gpg-agent option passed to 'configure' ]]] Index: configure.ac === --- configure.ac (revision 1005331) +++ configure.ac (working copy) @@ -551,33 +551,18 @@ dnl GPG Agent --- AC_ARG_WITH(gpg_agent, - AS_HELP_STRING([--with-gpg-agent], - [Enable use of GPG AGENT for auth credentials]), - [with_gpg_agent=$withval], - [with_gpg_agent=no]) - -AC_MSG_CHECKING([whether to look for GPG AGENT]) -if test $with_gpg_agent != no; then +AS_HELP_STRING([--with-gpg-agent], + [Enable use of GPG AGENT for auth credentials])) +AC_MSG_CHECKING([whether to support GPG-Agent]) +if test $with_gpg_agent = yes; then AC_MSG_RESULT([yes]) - if test $svn_enable_shared = yes; then -if test $APR_HAS_DSO = yes; then -if test $with_gpg_agent = yes; then - AC_MSG_RESULT([yes]) - AC_DEFINE([SVN_HAVE_GPG_AGENT], [1], -[Is GPG Agent support enabled?]) - SVN_GPG_AGENT_LIBS=-lgcrypt11 -fi -else - AC_MSG_ERROR([APR does not have support for DSOs]) -fi - else -AC_MSG_ERROR([--with-gpg-agent conflicts with --disable-shared]) - fi + AC_DEFINE([SVN_HAVE_GPG_AGENT], [1], +[Is GPG Agent support enabled?]) else AC_MSG_RESULT([no]) fi -AC_SUBST(SVN_GPG_AGENT_LIBS) +AC_SUBST(SVN_HAVE_GPG_AGENT) dnl GNOME Keyring ---
Re: [PATCH] gpg-agent configure fix.
Hi Alex, Alexander Thomas wrote: [[[ On the 'gpg-agent-password-store' branch: * configure.ac: (SVN_HAVE_GPG_AGENT): Defines when --with-gpg-agent option passed to 'configure' ]]] Committed the patch in r1005433. Thank You. -- Senthil Kumaran S http://www.stylesen.org/
Re: Performance optimization - svn_stringbuf_appendbyte()
New Revision: 997203
URL: http://svn.apache.org/viewvc?rev=997203view=rev
Log:
Merge r985037, r985046, r995507 and r995603 from the performance branch.
These changes introduce the svn_stringbuf_appendbyte() function, which has
significantly less overhead than svn_stringbuf_appendbytes(), and can be
used in a number of places within our codebase.
Hi Stefan2.
I have been wondering about the actual benefit of such tightly
hand-optimized code. Today I got around to giving it a quick spin.
In my first test, it made no difference whatsoever, because the
optimized code is never executed. The recent merge r1002898 of r1001413
from the performance branch introduced a bug:
- if (str-blocksize old_len + 1)
+ if (str-blocksize old_len + 1)
which totally disables the optimized code path.
Fixed in r1005437.
That solved, a loop doing a million simple appendbyte calls runs more
than twice as fast as calling appendbytes(..., 1). That's fantastic.
But what about that hand-optimization? I wrote a simple version of the
function, and called it 'appendbyte0':
svn_stringbuf_appendbyte0(svn_stringbuf_t *str, char byte)
{
if (str-blocksize str-len + 1)
{
str-data[str-len++] = byte;
str-data[str-len] = '\0';
}
else
svn_stringbuf_appendbytes(str, byte, 1);
}
Here are the results (see full patch attached):
Times: appendbytes appendbyte0 appendbyte (in ms)
run: 89 31 34
run: 88 30 35
run: 88 31 34
run: 88 30 34
run: 88 31 34
min: 88 30 34
This tells me that the hand-optimization is actually harmful and the
compiler does a 10% better job by itself.
Have I made a mistake?
What are the results for your system?
(I'm using GCC 4.4.1 on an Intel Centrino laptop CPU.)
- Julian
A comparative performance test of svn_stringbuf_appendbyte().
* subversion/libsvn_subr/svn_string.c
(svn_stringbuf_appendbyte0): New function, for comparative testing.
* subversion/tests/libsvn_subr/string-test.c
(test24): New test, measuring and printing a speed comparison.
(test_funcs): Add it.
--This line, and those below, will be ignored--
Index: subversion/libsvn_subr/svn_string.c
===
--- subversion/libsvn_subr/svn_string.c (revision 1005437)
+++ subversion/libsvn_subr/svn_string.c (working copy)
@@ -392,6 +392,29 @@ svn_stringbuf_ensure(svn_stringbuf_t *st
}
+/* Unoptimized version of svn_stringbuf_appendbyte(), for comparison. */
+void
+svn_stringbuf_appendbyte0(svn_stringbuf_t *str, char byte)
+{
+ /* In most cases, there will be pre-allocated memory left
+ * to just write the new byte at the end of the used section
+ * and terminate the string properly.
+ */
+ if (str-blocksize str-len + 1)
+{
+ str-data[str-len++] = byte;
+ str-data[str-len] = '\0';
+}
+ else
+{
+ /* we need to re-allocate the string buffer
+ * - let the more generic implementation take care of that part
+ */
+ svn_stringbuf_appendbytes(str, byte, 1);
+}
+}
+
+
/* WARNING - Optimized code ahead!
* This function has been hand-tuned for performance. Please read
* the comments below before modifying the code.
Index: subversion/tests/libsvn_subr/string-test.c
===
--- subversion/tests/libsvn_subr/string-test.c (revision 1003481)
+++ subversion/tests/libsvn_subr/string-test.c (working copy)
@@ -511,6 +511,50 @@ test23(apr_pool_t *pool)
return test_stringbuf_unequal(abc, abb, pool);
}
+void
+svn_stringbuf_appendbyte0(svn_stringbuf_t *str, char byte);
+
+static svn_error_t *
+test24(apr_pool_t *pool)
+{
+ apr_time_t start;
+ int t1, t2, t3;
+ int min_t1 = 99, min_t2 = 99, min_t3 = 99;
+ char byte = 'X';
+ int N = 200, TESTS = 5;
+ int i, j;
+
+ printf(Times: appendbytes appendbyte0 appendbyte (in ms)\n);
+ for (j = 0; j TESTS; j++)
+{
+ a = svn_stringbuf_create(, pool);
+ start = apr_time_now();
+ for (i = 0; i N; i++)
+svn_stringbuf_appendbytes(a, byte, 1);
+ t1 = (int)(apr_time_now() - start) / 1000;
+ if (t1 min_t1) min_t1 = t1;
+
+ a = svn_stringbuf_create(, pool);
+ start = apr_time_now();
+ for (i = 0; i N; i++)
+svn_stringbuf_appendbyte0(a, byte);
+ t2 = (int)(apr_time_now() - start) / 1000;
+ if (t2 min_t2) min_t2 = t2;
+
+ a = svn_stringbuf_create(, pool);
+ start = apr_time_now();
+ for (i = 0; i N; i++)
+svn_stringbuf_appendbyte(a, byte);
+ t3 = (int)(apr_time_now() - start) / 1000;
+ if (t3 min_t3) min_t3 = t3;
+
+ printf(run: %10d %10d %10d\n, t1, t2, t3);
+}
+ printf(min: %10d %10d %10d\n, min_t1, min_t2, min_t3);
+
+ return SVN_NO_ERROR;
+}
+
/*
Re: [PATCH] extend svn_subst_translate_string() to record whether re-encoding and/or line ending translation were performed (v. 2)
Daniel, sorry I haven't looked at this yet, and won't be able to today. - Julian On Sun, 2010-10-03 at 11:55 -0700, Daniel Trebbien wrote: On Fri, Oct 1, 2010 at 3:57 AM, Julian Foad julian.f...@wandisco.com wrote: Adds a public API function, svn_subst_translate_string2(), an extension of svn_subst_translate_string(), that has two, additional output parameters for determining whether re-encoding and/or line ending translation were performed. If we're going to add this functionality to _translate_string(), shouldn't we also add it to the other variants - _detranslate_string, _translate_cstring2, _stream_translated, _copy_and_translate4? I see you're adding the infrastructure into the (new) bodies of _translate_cstring2 and _stream_translated, just not (yey) exposing it in their APIs. I'm not sure. The set of 'translation' functions is already messy and it's not clear to me how useful this new functionality will be. I originally began working on svn_subst_translate_string2() because I could not find a combination of public API functions that would allow me to determine whether the line endings changed when a string was both re-encoded to UTF-8 and normalized to LF line endings. The most applicable, svn_subst_translate_string(), performs both translations without indicating whether it re-encoded the string or normalized a line ending. An alternative to extending svn_subst_translate_string() is to add a public API function that does the re-encoding and another that normalizes line endings. I think, however, that extending svn_subst_translate_string() is better because though the current implementation of svn_subst_translate_string() re-encodes, then normalizes line endings, the single API function allows for the possibility of a future improvement in efficiency as a result of performing both translations simultaneously. Attached are a revised patch and log message.
Re: Problem with external file
Philip Martin philip.martin at wandisco.com writes: Ideally these would be commands we could execute using the command line client. Thanks, Philip. I normally use TortoiseSVN so I will have to spend some time seeing if/how this occurs with the command line client. Maybe I can get something this weekend. Bob
Re: Performance optimization - svn_stringbuf_appendbyte()
Julian Foad julian.f...@wandisco.com writes: This tells me that the hand-optimization is actually harmful and the compiler does a 10% better job by itself. Have I made a mistake? What are the results for your system? (I'm using GCC 4.4.1 on an Intel Centrino laptop CPU.) On my system (Intel Core 2 Duo, GCC 4.3.2, 64bit), the difference is smaller, about 5%. -- Philip
Re: svn commit: r1003238 - in /subversion/trunk/subversion: svn/cl.h svn/propget-cmd.c svn/proplist-cmd.c svn/props.c tests/cmdline/prop_tests.py
On Thu, Oct 7, 2010 at 8:09 AM, Julian Foad julian.f...@wandisco.com wrote: Hi Paul. It's good to see a fix. I think, however, you could do this more simply and avoid some of the difficulties completely. Julian, Thanks as always for the review, comments inline. On Thu, 2010-09-30, pbu...@apache.org wrote: Author: pburba Date: Thu Sep 30 20:21:19 2010 New Revision: 1003238 URL: http://svn.apache.org/viewvc?rev=1003238view=rev Log: Fix issue #3721 'redirection of svn propget output corrupted with large property values'. And this change also fixes an EOL-style inconsistency, which was presumably happening consistently with property values of any size, doesn't it? Yes. I think this change also introduces a new, similar, EOL-style inconsistency, in a different place; see last review comment at end of email. You are right, there is a problem, though it might not be exactly what you think: If the property requires translation per svn_prop_needs_translation (i.e. it's an svn:* prop) we will already have run the property value through svn_subst_detranslate_string(), which gives us native line endings. So in these cases the only non-native newline will be the trailing newline after the propval: /* Add an extra newline to the value before indenting, so that * every line of output has the indentation whether the value * already ended in a newline or not. */ const char *newval = apr_psprintf(pool, %s\n, propval-data); ^^ If we return to the pre-r1003238 behavior of using printf, then multiline svn:* prop values (e.g. svn:mergeinfo) are passed to printf with native newlines and then printf tries (on my Windows box at any rate) to convert those eols to native, resulting in redirected output like this: Properties on 'iota':CRLF propnameCRLF subversion/branches/1.5.x:872364-874936CRCRLF subversion/branches/1.5.x-34184:874657-874741CRCRLF subversion/branches/1.5.x-34432:874744-874798CRCRLF . . So avoiding printf's eol conversion by writing the the out stream works better here. The difficulty with EOLs is that the native printf() family of functions typically converts to native EOLs, and the svn_stream_write() family doesn't, even when writing to svn_stream_for_stdout(). Therefore we have to be careful when mixing the two families. This change makes all the writes to stdout, by svn pg, via the svn_stream_t * that we already use to print the 'Properties on' header. Note that this stream *is* attached to stdout, but avoiding the mix of stream writes and printfs solves issue #3721 on Windows. It looks like the root of the corruption is also the mixing of native 'printf' functions with writes to a 'svn_stream_for_stdout()' stream. Do you agree that that's the case, and that therefore I should write a warning about this in the documentation of 'svn_stream_for_stdout()'? I agree that is what the symptoms point to. I couldn't definitely pinpoint the exact cause however. Perhaps not now, but we should consider whether we can eliminate the corruption by doing something to svn_stream_for_stdout() and/or the native 'stdout' stream. It might be possible by turning off some buffering that is happening, or something like that, or probably it could be done by implementing the stream_for_stdout as a stream class that forwards to the printf family of functions. What we probably should do now is avoid mixing printf with svn streams in this 'svn propget' code. From what I can see, there is little or no reason why svn_cl__propget() was using an svn stream for some of its printing at all. It looks like it would be much simpler to remove that stream (the variable called 'out') and just use printf-style functions throughout, as is done for 'proplist' and most of the rest of the UI (except for 'cat' and 'blame'). Note that we have svn_cmdline_printf() which is a handy encoding-converting variant of the native 'printf' family. Does that sound good? Assuming there was never a reason to mix printfs with stream writes in the first place, then yes, this would be a much simpler fix. As mentioned above though, we will need to deal with printf's conversion to native eols when the string it's printing already has them. Paul If so, then all the comments below become moot. (I wrote some of them before I figured all this out, and they may still be relevant if I'm wrong about some of this.) * subversion/svn/cl.h (svn_cl__print_prop_hash): Support printing to a passed in svn_stream_t *. * subversion/svn/propget-cmd.c (print_properties): Make the header's line endings native. Print the property names and values to the same svn_stream_t * we already use print the headers to. * subversion/svn/proplist-cmd.c (proplist_receiver, svn_cl__proplist): Update calls to svn_cl__print_prop_hash(), keeping old behavior. * subversion/svn/props.c (svn_cl__print_prop_hash): Support
Re: Problem with external file
On Thu, Oct 7, 2010 at 9:09 AM, Bob Fletcher robert.h.fletc...@ieee.org wrote: Philip Martin philip.martin at wandisco.com writes: Ideally these would be commands we could execute using the command line client. Of even a script with these commands in it (though that isn't always feasible). Thanks, Philip. I normally use TortoiseSVN so I will have to spend some time seeing if/how this occurs with the command line client. Maybe I can get something this weekend. Thanks. By the way, we aren't trying to make life harder on you, or introduce some difficult-to-clear bar for getting bugs fixed. The developers as just hard at work on actually shipping 1.7, so doing the bits you can helps keep us from getting too distracted, and also helps get your bug fixed sooner. -Hyrum (Note to everybody else: Bob's decision to start testing 1.7, and report bugs here, ensures that his bugs are known and (hopefully) addressed before 1.7 ships. Interested parties would do well to follow his example. :)
[RFC] WC APIs for adding and copying
I'm planning to remove/deprecate svn_wc_add4(), the many-headed beast, and provide replacement functions that have more focused tasks. I reached this point from trying to understand the different kinds of add and copy in order to sort out the operation depth field for the NODES table. I drew out the call graph of all WC add and copy calls; it's attached as a PDF and visible at https://docs.google.com/drawings/edit?id=1Up_5tvnKjShQL2F8GGtoIefiBDp5k4ODf1_ZxrgBJ1Uhl=en. Plan: * Remove svn_wc_add4(). Keep the deprecated svn_wc_add3() for backward compatibility. Probably re-implement it in terms of the new simpler functions. * For an add without 'copyfrom', a new function: svn_wc_add[4|_tree|something](depth) # any node kind * For a repo-WC copy: svn_wc_add_repos_file4() # existing function svn_wc_add_repos_dir(depth) # new function * For a WC-to-WC copy: svn_wc_copy3()# existing function * Reorganize svn_wc_copy3() internally: If the target should be a simple add (if the source is itself a simple add or from a different repos), call the simple-add function. Copying a repository node from the WC Any thoughts? - Julian
Re: [RFC] WC APIs for adding and copying
On 10/07/2010 03:34 PM, Julian Foad wrote: I'm planning to remove/deprecate svn_wc_add4(), the many-headed beast, and provide replacement functions that have more focused tasks. [...] Any thoughts? Anything that can be done to make that multi-headed beast more maintainable would be a welcome change, IMO. Goforit. -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: Problem with external file
(Note to everybody else: Bob's decision to start testing 1.7, and report bugs here, ensures that his bugs are known and (hopefully) addressed before 1.7 ships. Interested parties would do well to follow his example. :) On this note. I'd been testing 1.7 using the binaries at: http://nightlybuilds.tortoisesvn.net/latest/win32/small/ However my hardware recently rolled over and I'm on Win7 64-bit and can't get these ones to work: http://nightlybuilds.tortoisesvn.net/latest/x64/small/ It fails with The application was unable to start correctly (0xc07b). This seems to be a fairly unhelpful message. 1. Is anyone able to get these binaries to work? 2. Are there other 64-bit binaries we can use? It would be good to be able to keep experimenting with Subversion without everyone doing so having to build it themselves. -- Talden
Re: [RFC] WC APIs for adding and copying
On Thu, Oct 7, 2010 at 15:34, Julian Foad julian.f...@wandisco.com wrote: ... * For a repo-WC copy: svn_wc_add_repos_file4() # existing function svn_wc_add_repos_dir(depth) # new function Please use copy in their name to better reflect the operation. Let's never use the term add-with-history and stick to just copy. Note that svn_wc_add_repos_file3() would then be implemented in terms of (say) svn_wc_copy_repos_file(). ... Cheers, -g
Re: [PATCH] fix @deprecated tags in header comments
Ping. This patch has received no comments. Gavin Beau Baumanis On 27/09/2010, at 7:23 AM, Dani Church wrote: This patch is a fix for some of the @deprecated tags in the Doxygen comments. It standardizes them so that the phrasing Provided for backward compatibility with the 1.x API refers to the LAST version for which the function was valid rather than the first. [[[ * subversion/include/svn_fs.h, subversion/include/svn_diff.h, subversion/include/svn_mergeinfo.h, subversion/include/svn_repos.h, subversion/include/svn_io.h, subversion/include/svn_wc.h, subversion/include/svn_client.h: Fix @deprecated tags in Doxygen comments. ]]] svn-deprecation-comments.patch
[RFC] Diff (blame) optimization: how to go forward
Hi all, This is a follow-up to the WIP-patches I posted last week [1], which are about improving performance of svn_diff (and therefor also blame on the client-side), especially for large files. To summarize: the idea was (is) to strip off the identical prefix and suffix, and then letting the existing diff algorithm work on the remainder. As stated in [2], I ran into some difficulties to get the exact same result of the existing diff algorithm (because of too eager suffix stripping). I've spent some time searching for a perfect solution, but I can't find one. So now I'm stuck and would like to have some feedback on what to do with it. First: the thing works, and it can reduce the time needed for svn_diff by 20% up to 80% for large files (depending on the distribution of the changes). An extreme example is a reduction from ~150ms to ~30ms for a 6-lines file with a small number of lines changed (say up to 100) located close together (so lots of identical prefix/suffix). Blame gets similar benefits, *if the server is fast enough*. I used svnserve built from Stefan^2's performance branch to test this. A normal svnserve with FSFS isn't really fast enough (unless maybe with an SSD, but I don't have one here) to serve up the 2500 revisions of the large file quickly enough. But the performance-enhanced-svnserve, with a hot cache filled with the necessary full-texts, is definitely fast enough to make the time of blame completely dependent on the client-side performance. Conclusion: the diff optimization we're talking about is much more useful for blame together with a server with the full-text membuffer caching of the performance branch. Now, the reason why I'm stuck: suffix scanning sometimes strips a little bit too much (see [2] for full explanation). In this case the resulting diff is still correct, but it's not as nice for the user. As I noted in [2], GNU diff also has this problem, but the user can help a little bit by specifying a large enough number of prefix/suffix-lines to keep (--horizon-lines). So, what to do? - I have not found a better solution than to use some form of horizon-lines to get the nice result, but still have a similar performance improvement (unless if I leave out suffix stripping entirely, only strip prefix, but that halves the power of the optimization). - I've tested with keeping up to 50 suffix-lines. It didn't have the slightest impact on the performance improvement (we're talking about stripping away 1000's of lines). This fixed all real-life examples I could find/test (diff/blame output is precisely identical to original svn). If I kept only 20 lines, I still ran into some differences (30 lines was enough for my example file, but I took it up to 50 to be sure). - I would like to avoid leaving the decision to the user to come up with an adequate number of suffix-lines-to-keep. So I'd like to just hardcode some value, say 50, or even 100. I think this would give good (=identical to original svn) diff/blame results in all but the most extreme cases. With suffix-lines-to-keep=50, you'd need to insert a block of text that has its last 50 lines identical to the 50 lines preceding the insertion point, to mess up the diff result. - If really necessary, we could say default=50, but it can be overridden by the user with another -x option. So the main question is: is such a change in behavior (unlikely to have an impact in most realistic cases) acceptable for this kind of performance improvement? Other options: - Come up with a clever solution to fix the optimization properly (but I don't know if that's possible - suggestions welcome :-)). - Throw this optimization away, and take a look at something like Patience diff (supposed to be fast as well, and give nice results). However, I don't know Patience diff well enough to code up something, or even to judge whether it would give good results in all (or most) cases. - Just throw it away and don't look back :-) - Something else? Cheers, -- Johan [1] http://svn.haxx.se/dev/archive-2010-10/0032.shtml [2] http://svn.haxx.se/dev/archive-2010-10/0050.shtml
Re: [PATCH] Fix svnrdump test 27 on Windows
Hi Gavin, Gavin Beau Baumanis writes: Ping. This patch has received no comments. This patch has been committed. See r1003064- the directory baton is now allocated in `eb-pool`. I hoped that it would fix test 27, but it didn't- that is fixed in r1005035/ r1005050. Thanks for picking this up. -- Ram
