Re: [PATCH] issue #3469: tree conflict under a directory external
'Daniel Shahaf' wrote on Mon, Mar 28, 2011 at 00:16:49 +0200:
Bert Huijben wrote on Sun, Mar 27, 2011 at 19:26:50 +0200:
and there are easier (and more
performant) ways to check if a node is part of a working copy (e.g. helpers
to check if a path is switched/a wc root).
Could you elaborate on these please? I looked through the header files
and amn't sure which helpers you're referring to.
New patch, including your previous feedbacks modulo the above point:
[[[
Resolve issue #3469 (tree conflict under a directory external).
Review by: rhuijben
* subversion/libsvn_client/merge.c
(merge_cmd_baton_t): Add TARGET_WCROOT_ABSPATH member.
(do_merge): Initialize new member.
(merge_file_added):
Skip files that belong to externals or to disjoint working copies.
Exploit knowledge of libsvn_wc implementations details (queries on
directories are faster) to make the check cheaper.
TODO: remove @XFail decorator
(from tree_conflict_tests.py 22)
]]]
[[[
Index: subversion/libsvn_client/merge.c
===
--- subversion/libsvn_client/merge.c(revision 1086475)
+++ subversion/libsvn_client/merge.c(working copy)
@@ -249,6 +249,8 @@ typedef struct merge_cmd_baton_t {
versioned dir (dry-run only) */
const char *target_abspath; /* Absolute working copy target of
the merge. */
+ const char *target_wcroot_abspath; /* Absolute path to root of wc that
+ TARGET_ABSPATH belongs to. */
/* The left and right URLs and revs. The value of this field changes to
reflect the merge_source_t *currently* being merged by do_merge(). */
@@ -1555,6 +1557,32 @@ merge_file_added(const char *local_dir_abspath,
if (tree_conflicted)
*tree_conflicted = FALSE;
+ /* Easy out: not the same working copy. (So, a disjoint working copy or
+ an external.) */
+ {
+const char *mine_wcroot_abspath;
+
+/* ### White-box optimization:
+
+ The code knows that MINE_ABSPATH is not a directory (it's an added
+ file), and we know that internally libsvn_wc queries are faster for
+ directories (this is an implementation detail). Therefore, query for
+ the wcroot of the containing directory of MINE_ABSPATH.
+
+ (We rely on the implementation detail only for performance, not for
+ correctness; under any implementation it would be valid to query for
+ the parent's wcroot.)
+ */
+SVN_ERR(svn_wc_get_wc_root(mine_wcroot_abspath, merge_b-ctx-wc_ctx,
+ svn_dirent_dirname(mine_abspath, scratch_pool),
+ scratch_pool, scratch_pool));
+if (strcmp(mine_wcroot_abspath, merge_b-target_wcroot_abspath))
+ {
+*content_state = svn_wc_notify_state_obstructed;
+return SVN_NO_ERROR;
+ }
+ }
+
/* Apply the prop changes to a new hash table. */
file_props = apr_hash_copy(subpool, original_props);
for (i = 0; i prop_changes-nelts; ++i)
@@ -8653,6 +8685,9 @@ do_merge(apr_hash_t **modified_subtrees,
merge_cmd_baton.target_missing_child = FALSE;
merge_cmd_baton.reintegrate_merge = reintegrate_merge;
merge_cmd_baton.target_abspath = target_abspath;
+ SVN_ERR(svn_wc_get_wc_root(merge_cmd_baton.target_wcroot_abspath,
+ ctx-wc_ctx, merge_cmd_baton.target_abspath,
+ pool, subpool));
merge_cmd_baton.pool = subpool;
merge_cmd_baton.merge_options = merge_options;
merge_cmd_baton.diff3_cmd = diff3_cmd;
]]]
RE: svn commit: r1085464 - /subversion/trunk/subversion/svnrdump/load_editor.c
-Original Message- From: hwri...@apache.org [mailto:hwri...@apache.org] Sent: vrijdag 25 maart 2011 17:52 To: comm...@subversion.apache.org Subject: svn commit: r1085464 - /subversion/trunk/subversion/svnrdump/load_editor.c Author: hwright Date: Fri Mar 25 16:52:10 2011 New Revision: 1085464 URL: http://svn.apache.org/viewvc?rev=1085464view=rev Log: A bit of work on issue #3844. The test still fails, but at least it's not with an assertion. * subversion/svnrdump/load_editor.c (remove_node_props): Attempt to implement for non-newly-added nodes. This patch needs a second ra session to work properly. (ra_local, ra_neon and ra_serf happen to work ok, but the session is busy with a different operation here). See the svnserve failures. Bert
Re: Performance benchmarks
Forgot to add: this was with the repository via svnserve on localhost. Johan On Tue, Mar 29, 2011 at 12:16 AM, Johan Corveleyn jcor...@gmail.com wrote: Hi Mark, Here is another data point, for my old (t)rusty Windows XP (32-bit) this time, on a system with a pretty slow hard disk (5.4k rpm), 1.83 GHz Intel T2400 cpu, 3 GB RAM. I must say the results look very good for 1.7 (r1086021) compared to 1.6.16 on this system. Especially for the Folder tests and the Binaries tests (rather: you can see how much it hurts sometimes to use 1.6.x on this kind of system, when working with working copies with lots of folders). Apart from Bin-commit and Bin-rm, I can't see a significant performance degradation within your test-suite on my system. Both test runs were run with antivirus enabled (AVG free). I also did a run of 1.7 with AV disabled, but it made no significant difference. I rebooted between each test run just to be sure about disk caching effects. Oh, and I made my 1.7 build with Release configuration. At least on Windows, I know there is a significant performance difference between Debug builds and Release builds (that became clear during my perf-work on svn diff). Best to compare release builds with release builds, I think... Cheers, -- Johan
Re: Performance benchmarks
On Sun, Mar 27, 2011 at 4:51 AM, Mark Phippard markp...@gmail.com wrote: On Fri, Mar 25, 2011 at 1:33 PM, Mark Phippard markp...@gmail.com wrote: I have been working on a framework for writing tests to record performance. I have something good enough to share: https://ctf.open.collab.net/sf/projects/csvn It is pretty easy to add new tests if you have ideas on more tests you think we should add. I think I have pretty good coverage of the major functions. The wiki on the site I linked to above has details on how I have constructed the current tests. I am going to put out a call to users for feedback and try to get more people to run the tests and record results. I am not claiming these are anything definitive or even that we will use them to help us make the release decision, but I think it is a start on coming up with some reproducible tests that people can run easily. If after people look at and run the tests they think they are useful or can be tweaked to be useful, then great. If not, then at least I got to write some code for a change :) The tests are written in Java because that is what I know and it gives me good cross platform coverage. However, the Java just drives the command line so all you need to do is have the svn command line in your PATH and that is what it uses for all the work. These tests are showing some interesting results. * There are a number of cases where 1.7 shows large performance gains over 1.6. For example, a WC with a lot of folders on Windows: https://ctf.open.collab.net/sf/wiki/do/viewPage/projects.csvn/wiki/FolderTests * Checkout seems to be the biggest remaining hotspot where 1.7 tends to be slower, although commit seems to still be slower too. * Delete and move are slower than I would have expected. Is this because we can actually delete the folder now, as opposed to waiting until commit time? I actually thought that made would make it faster as I thought the folder used to get deleted and then put back (maybe just in IDE's). * There is something seriously bad going on with Anti-Virus on my Windows laptop (Symantec Endpoint Protection). I noticed the times for commit were way out of whack so I configured my A/V to ignore the folder where the benchmarks were running. When I ran the tests again, the performance of 1.7 was waaay better. I am about 95% certain the A/V is the problem but I need to make sure. While it was great to see the good performance, it is still a big concern to see that A/V might be having an even bigger impact than it has before. 1.6 also showed performance improvements but they were less dramatic than some of the 1.7 commands. Stefan Küng asked me to add svnversion to the benchmark and I have done so in the latest release. I would love to see someone do some tests with the WC on local disk vs network mount (1.6 and 1.7). I tried to do it using some virtual machines I have access to at CollabNet. The problem is that the connection of these boxes to the NetApp with our home folders is too slow. Some of the checkouts (even using 1.6) were running for an hour and I finally killed the test. Anyway, overall I am encouraged by the results I am seeing so far. I look forward to more people running the tests and sharing the results. Attached are my results from the benchmark suite. I've provided two versions of the 1.7.0 run. One was an older build (prior to the optimizations), and the last one was a recent build. These were all run over file:///. I'll try and see if I can re-run it over svn://. Cheers, Daniel B. === TEST RESULTS == SVN Version: 1.7.0-dev Tests: Basic Tests Action TimeMillis -- - - Checkout: 0:03.695 3695 Update: 0:03.364 3364 Switch: 0:00.266 266 Proplist: 0:00.07171 Status: 0:00.09999 Commit: 0:01.151 1151 Tests: Merge Tests Action TimeMillis -- - - Merge-all: 0:02.672 2672 Merge-revert: 0:01.229 1229 Merge-synch: 0:02.880 2880 Merge-reintegrate: 0:04.129 4129 Tests: Folder Tests Action TimeMillis -- - - Folder-co: 0:54.932 54932 Folder-st: 0:01.143 1143 Folder-ci: 0:08.488 8488 Folder-up: 0:03.090 3090 Tests: Binaries Tests Action TimeMillis -- - - Bin-co: 2:13.671133671 Bin-up-r25: 0:02.532 2532 Bin-sw: 2:09.494129494 Bin-cleanup: 0:00.04646 Bin-rm: 0:03.811 3811
Serf failures with trunk
I'm getting assert failures while testing serf performance. Typically ../src/subversion/libsvn_ra_serf/update.c:622: (apr_err=235000) svn: E235000: In file '../src/subversion/libsvn_ra_serf/update.c' line 622: assertion failed (! dir-ref_count) Aborted or: svn: E200014: Checksum mismatch for 'xx/notes/obliterate/fspec-cc1/cc1-dir-ops.svg': expected: 0624a5d5bd362105a341d8e5e693238e actual: d5e94a461c7a84ff5e41f7ab037dad10 My test setup is: - create a repository, - import a subversion trunk wc - start apache to serve the repository My test is simply to export or check-out that repository. The exact error seems to vary from run to run, and occasionally it will run to completion. I get the error with both BDB and FSFS repositories, with 1.6 and 1.7 mod_dav_svn, on localhost and over a LAN. I don't get the error with neon. Can anyone reproduce this? -- Philip
Re: Bug in svnrdump (trunk@1085375) -- malformed dump file with multiple properties
I've taken a shot at a regression test in r1086497. However, I think we may have a larger problem here: The svn_delta_editor_t API allows change_dir_prop() calls to be transmitted only after all directory-children of a directory have been transmitted [1]. How can we generate a dumpfile given that input? On the one hand, we don't want to buffer entire subtrees rooted in D while we're waiting for D's properties. On the other hand, can we dump the properties of D only after dumping the entire subtree rooted in D? I expect the 'svnadmin load parser to want to see an entry for D before seeing an entry for any of its children. Which, yes, leaves the option to dump first a skeleton entry for D, then dump D's subtree, and finally dump *another* Node-path: D entry --- but that smells ugly, if not in outright violation of the dumpfile format. Thoughts? Daniel [1] * 4. A producer must call @c change_dir_prop on a directory either *before opening any of the directory's subdirs or after closing *them, but not in the middle. neil.win...@bt.com wrote on Mon, Mar 28, 2011 at 13:26:46 +0100: On 25 March 2011 at 19:19, Hyrum K Wright wrote On Fri, Mar 25, 2011 at 2:11 PM, C. Michael Pilato cmpil...@collab.net wrote: On 03/25/2011 11:31 AM, Hyrum K Wright wrote: Neil, Thanks for the report. In attempting to reproduce, I wasn't even able to get as far as comparing the dumpfiles, as svnrdump asserted somewhere in the process. I opened issue 3844[1] to track this bug, and committed an XFailing test in r1085428. I also marked the issue as important for a 1.7.0 release. -Hyrum [1] http://subversion.tigris.org/issues/show_bug.cgi?id=3844 See r1085523. Thanks, Mike. Neil, when you get a chance, could you test a r1085523 and see if it fixes your problem? Cheers, -Hyrum Hi guys, Sorry, but no cigar :( This probably fixes the assertion found by Hyrum, but doesn't address the original issue. The current test case added to the test suite addresses loading multiple propedits. That's good, but it's not sufficient. The heart of the issue is that a transaction with multiple propedits to a single path is *dumped* incorrectly. So for an original dump representation like this: --- Node-path: Node-kind: dir Node-action: change Prop-content-length: 42 Content-length: 42 K 3 foo V 3 bar K 3 bar V 3 baz PROPS-END --- The svnrdump output for the same transaction comes out like this: --- Node-path: Node-kind: dir Node-action: change Prop-delta: true Prop-content-length: 26 Content-length: 26 K 3 foo V 3 bar PROPS-END Prop-delta: true Prop-content-length: 26 Content-length: 26 K 3 bar V 3 baz PROPS-END --- Note the second orphaned prop-delta. It would be great if svnrdump produced exactly the same output as svnadmin (it'd make the test assertions nice and easy), but from a bit of experimentation (I haven't read the svnadmin code here) I don't believe that it's necessary for the two prop-deltas actually to be merged for the dumpfile to be valid. So if it's easier to repeat the preceding node-path information for each prop-delta then I guess that would be OK, as long as the net result is the same. Thanks, Neil
Re: [PATCH] Bump serf version in get-deps.sh
Thanks, r1086537.
Arwin Arni wrote on Tue, Mar 29, 2011 at 14:53:00 +0530:
Hi All,
The minimal serf version that our trunk requires is 0.7.1. However, the
get-deps.sh script still fetches 0.7.0. Here's a patch to bump this
version. This makes it easier for first-timers to compile and build
subversion.
Regards,
Arwin Arni
Index: get-deps.sh
===
--- get-deps.sh (revision 1086501)
+++ get-deps.sh (working copy)
@@ -26,7 +26,7 @@
APR=apr-1.3.9
APR_UTIL=apr-util-1.3.10
NEON=neon-0.29.5
-SERF=serf-0.7.0
+SERF=serf-0.7.1
ZLIB=zlib-1.2.5
SQLITE_VERSION=3.7.3
SQLITE=sqlite-amalgamation-$SQLITE_VERSION
* get-deps.sh:
Bump serf version.
Patch by: Arwin Arni arwin{_AT_}collab.net
Re: Performance benchmarks
On 29/03/11 01:33, Greg Stein wrote: On Mon, Mar 28, 2011 at 18:51, John Beranek j...@redux.org.uk wrote: On 28/03/2011 23:45, Greg Stein wrote: On Mon, Mar 28, 2011 at 17:42, John Beranek j...@redux.org.uk wrote: On 25/03/2011 17:33, Mark Phippard wrote: Hi, I have been working on a framework for writing tests to record performance. I have something good enough to share: May I make an observation about these benchmarks...? When I provided some benchmarks that included 'checkout' tests I was specifically asked to make tests that separate WC and RA functionality. I did this, released results, and the (portable) benchmark code. Now Mark has released a new set of benchmarks, which don't separate WC and RA functionality. No one has (yet) noted this fact. ;) I think your benchmarks are going to be more helpful for us to locate hotspots and get them fixed. Mark's seem more high-level, for policy-making rather than coding. Did your benchmark scripts get checked in? (I've been out a couple weeks and may have missed that) And whether they did or not, would you want commit access to get them committed, and/or continue work on them within the svn repository? I checked them into a Git repository, both for ease of repository creation, and for ease of cloning. Of course, hosting SVN tools in Git may be seen as sacrilegious by some... ;) Heh. I certainly don't mind. They just aren't going to get a lot of attention outside of our own repository, I think. At a minimum, what's the URL? https://github.com/jberanek/svn_scripts I'm certainly not averse to putting the script into the SVN repository if anyone thinks it's worthwhile though. I think I'd need to at least add some usage information before doing so. Cheers, John. -- John Beranek To generalise is to be an idiot. http://redux.org.uk/ -- William Blake smime.p7s Description: S/MIME Cryptographic Signature
Re: Serf failures with trunk
On Tue, Mar 29, 2011 at 6:02 AM, Philip Martin philip.mar...@wandisco.com wrote: Can anyone reproduce this? I ran the test results I posted using Serf trunk on a Windows box. Paul Burba did as well. -- Thanks Mark Phippard http://markphip.blogspot.com/
Re: Performance benchmarks
On Sunday 27 March 2011 12:21 AM, Mark Phippard wrote: I would love to see someone do some tests with the WC on local disk vs network mount (1.6 and 1.7). I tried to do it using some virtual machines I have access to at CollabNet. The problem is that the connection of these boxes to the NetApp with our home folders is too slow. Some of the checkouts (even using 1.6) were running for an hour and I finally killed the test. I have run the tests on RHEL-5.3 x86_64 with subversion 1.6.9 and 1.7.0-dev(r1086490). It covers the cases *WC on local disk vs WC on network mount*. Repository access layer: file:// Attached the results. Thanks Regards, Vijayaguru === TEST RESULTS == SVN Version: 1.6.9 Tests: Basic Tests Action TimeMillis -- - - Checkout: 0:02.705 2705 Update: 0:04.998 4998 Switch: 0:01.002 1002 Proplist: 0:00.07272 Status: 0:00.04949 Commit: 0:00.876 876 svnversion: 0:00.05353 Tests: Merge Tests Action TimeMillis -- - - Merge-all: 0:02.005 2005 Merge-revert: 0:01.985 1985 Merge-synch: 0:02.009 2009 Merge-reintegrate: 0:03.007 3007 Tests: Folder Tests Action TimeMillis -- - - Folder-co: 1:02.765 62765 Folder-st: 0:02.889 2889 Folder-ci: 0:02.043 2043 Folder-up: 0:00.920 920 svnversion: 0:00.523 523 Tests: Binaries Tests Action TimeMillis -- - - Bin-co: 0:52.800 52800 Bin-up-r25: 0:51.998 51998 Bin-sw: 0:50.002 50002 Bin-cleanup: 0:00.995 995 Bin-rm: 0:00.527 527 Bin-st: 0:00.121 121 Bin-commit: 0:01.351 1351 Bin-mv: 0:00.999 999 Bin-st-mv: 0:00.121 121 Bin-commit: 0:01.881 1881 svnversion: 0:00.109 109 === END RESULTS == Total execution time: 4:34.182274182 Results in wiki format: Basic Tests: | 1.6.9 | r | 0:02.705 | 0:04.998 | 0:01.002 | 0:00.072 | 0:00.049 | 0:00.876 | 0:00.053 Merge Tests: | 1.6.9 | r | 0:02.005 | 0:01.985 | 0:02.009 | 0:03.007 Folder Tests: | 1.6.9 | r | 1:02.765 | 0:02.889 | 0:02.043 | 0:00.920 | 0:00.523 Binaries Tests: | 1.6.9 | r | 0:52.800 | 0:51.998 | 0:50.002 | 0:00.995 | 0:00.527 | 0:00.121 | 0:01.351 | 0:00.999 | 0:00.121 | 0:01.881 | 0:00.109 === TEST RESULTS == SVN Version: 1.6.9 Tests: Basic Tests Action TimeMillis -- - - Checkout: 0:37.371 37371 Update: 0:27.977 27977 Switch: 0:01.020 1020 Proplist: 0:00.914 914 Status: 0:00.528 528 Commit: 0:01.517 1517 svnversion: 0:00.236 236 Tests: Merge Tests Action TimeMillis -- - - Merge-all: 0:09.064 9064 Merge-revert: 0:03.895 3895 Merge-synch: 0:09.111 9111 Merge-reintegrate: 0:16.029 16029 Tests: Folder Tests Action TimeMillis -- - - Folder-co: 10:40.137640137 Folder-st: 0:10.723 10723 Folder-ci: 0:11.918 11918 Folder-up: 0:13.834 13834 svnversion: 0:07.266 7266 Tests: Binaries Tests Action TimeMillis -- - - Bin-co: 47:29.106 2849106 Bin-up-r25: 5:05.970305970 Bin-sw: 47:08.014 2828014 Bin-cleanup: 0:03.934 3934 Bin-rm: 0:06.549 6549 Bin-st: 0:03.297 3297 Bin-commit: 0:06.153 6153 Bin-mv: 0:12.991 12991 Bin-st-mv: 0:02.509 2509 Bin-commit: 0:06.494 6494 svnversion: 0:00.243 243 === END RESULTS == Total execution time:1:56:15.240 6975240 Results in wiki format: Basic Tests: | 1.6.9 | r | 0:37.371 | 0:27.977 | 0:01.020 | 0:00.914 | 0:00.528 | 0:01.517 | 0:00.236 Merge Tests: | 1.6.9 | r | 0:09.064 | 0:03.895 | 0:09.111 | 0:16.029 Folder Tests: | 1.6.9 | r | 10:40.137 |
Re: Performance benchmarks
Thanks! All results have been added to the wiki. On Tue, Mar 29, 2011 at 11:36 AM, vijay vi...@collab.net wrote: On Sunday 27 March 2011 12:21 AM, Mark Phippard wrote: I would love to see someone do some tests with the WC on local disk vs network mount (1.6 and 1.7). I tried to do it using some virtual machines I have access to at CollabNet. The problem is that the connection of these boxes to the NetApp with our home folders is too slow. Some of the checkouts (even using 1.6) were running for an hour and I finally killed the test. I have run the tests on RHEL-5.3 x86_64 with subversion 1.6.9 and 1.7.0-dev(r1086490). It covers the cases *WC on local disk vs WC on network mount*. Repository access layer: file:// Attached the results. Thanks Regards, Vijayaguru -- Thanks Mark Phippard http://markphip.blogspot.com/
Re: svn commit: r1085464 - /subversion/trunk/subversion/svnrdump/load_editor.c
On 03/29/2011 04:50 AM, Bert Huijben wrote: This patch needs a second ra session to work properly. (ra_local, ra_neon and ra_serf happen to work ok, but the session is busy with a different operation here). See the svnserve failures. FIXED: Sendingtrunk/subversion/svnrdump/load_editor.c Sendingtrunk/subversion/svnrdump/load_editor.h Sendingtrunk/subversion/svnrdump/svnrdump.c Transmitting file data ... Committed revision 1086735. Thanks for catching that, Bert. -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
Re: Bug in svnrdump (trunk@1085375) -- malformed dump file with multiple properties
On 03/28/2011 08:26 AM, neil.win...@bt.com wrote: Sorry, but no cigar :( This probably fixes the assertion found by Hyrum, but doesn't address the original issue. To simplify things, I've repurposed issue #3844[1] to track the problem that Hyrum was solving, and opened a new issue #3847[2] for this issue. -- C-Mike [1] http://subversion.tigris.org/issues/show_bug.cgi?id=3844 [2] http://subversion.tigris.org/issues/show_bug.cgi?id=3847 -- C. Michael Pilato cmpil...@collab.net CollabNet www.collab.net Distributed Development On Demand signature.asc Description: OpenPGP digital signature
the retry loop in svn_io_file_lock2()
Why does svn_io_file_lock2() have a retry loop? The reason given in the code is that some consumer of libsvn_fs wants to run a critical section while holding a lock in each of two FSFS filesystems. In that case, why isn't the fix just to tell the consumer to, when wanting to lock N filesystems, to always lock them in a well-defined order? Concretely: sort the filesystems by their UUID lexicographically.
