Re: place of svnrdump
Hi Neels, Neels J Hofmeyr writes: On a side note, svnsync happens to be relatively slow. I tried to svnsync the ASF repos once (for huge test data). The slowness of svnsync made it practically unfeasible to pull off. I ended up downloading a zipped dump and 'svnadmin load'ing that dump. Even with a zipped dump already downloaded, 'unzip | svnadmin load' took a few *days* to load the 950.000+ revisions. (And someone rebooted that box after two days, halfway through, grr. Took some serious hacking to finish up without starting over.) Yeah, we had a tough time obtaining the complete undeltified ASF dump for testing purposes as well. So, that experience tells me that svnsync and svnadmin dump/load aren't close to optimal, for example compared to a straight download of 34 gigs that the ASF repos is... Anything that could speed up a remote dump/load process would probably be good -- while I don't know any details about svnrdump. I just benchmarked it recently and found that it dumps 1 revisions of the ASF repository in 106 seconds: that's about 94 revisions per second. It used to be faster than `svnadmin` in an older benchmark: I'll work on perf issues this week. I estimate that it should be possible to get it to dump at ~140 revisions/second. @Daniel and others: I'd recommend a feature freeze. I'm currently profiling svnrdump and working on improving especially the I/O profile. My two cents: Rephrasing everything into the dump format and back blows up both data size and ETA. Maybe a remote backup mechanism could even break loose from discrete revision boundaries during transfer/load... I've been thinking about this too: we'll have to start attacking the RA layer itself to make svnrdump even faster. The replay API isn't optimized for this kind of operation. P.S.: If the whole ASF repos were a single Git WC, how long would that take to pull? (Given that Git tends to take up much more space than a Subversion repos, I wonder.) The gzipped undeltified dump of the complete ASF repository comes to about 25 GiB and it takes ~70 minutes to import it into the Git object store using a tool which is currently under development in Git. Thanks to David for these statistics. Cloning takes as long as it takes to transmit this data. After a repack, it'll probably shrink in size, but that's besides the point. Git was never designed to handle this- each project being a separate repository would be a fairer comparison. Even linux-2.6.git contains just 210887 revisions, and it tests Git's limits. -- Ram
[PATCH] Use neon's system proxy detection if not explicitly specified
Hi All,
[[[
Use Neon's system_proxy autodetection if no proxy has been explicitly
specified.
]]]
As a notebook user I'm of course using it on different
locations/networks and one thing rather annoying is that subversion only
cares for it's own configuration file when it comes to proxy
connections.
Since version 0.29.0 of libneon, it features a ne_session_system_proxy
API, which allows to transparently have the proxy settings filled, based
on what you have configured in your gnome/kde session (it uses libproxy
in the background for this). Should you happen to run subversion on any
different kind of session, is is falling back to using envvar.
I've been working with subversion the last few days with this
modification and it served me a lot, as I do only have to change my
gnome session's proxy settings now. svn is inheriting them, avoiding the
need to configure this file in plus (one location to rule them all!).
I'd be really glad if you would consider merging this patch (I wrote it
initially for 1.6.12, then later ported to svn trunk, which is slightly
different... I only tested the 1.6.12 version of the patch!).
Both patches are attached here for reference.
Best regards,
Dominique
Index: build/ac-macros/neon.m4
===
--- build/ac-macros/neon.m4 (revision 1001661)
+++ build/ac-macros/neon.m4 (working copy)
@@ -96,6 +96,11 @@
[Define to 1 if you have Neon 0.28 or later.])
fi
+ if test -n [`echo $NEON_VERSION | $EGREP '^0\.(29|3[0-9])\.'`] ; then
+AC_DEFINE_UNQUOTED([SVN_NEON_0_29], [1],
+ [Define to 1 if you have Neon 0.29 or later.])
+ fi
+
for svn_allowed_neon in $NEON_ALLOWED_LIST; do
if test -n `echo $NEON_VERSION | grep ^$svn_allowed_neon` ||
test $svn_allowed_neon = any; then
Index: subversion/libsvn_ra_neon/session.c
===
--- subversion/libsvn_ra_neon/session.c (revision 1001661)
+++ subversion/libsvn_ra_neon/session.c (working copy)
@@ -907,7 +907,18 @@
}
#endif
}
+#ifdef SVN_NEON_0_29
+else
+ {
+/* If we do not have any proxy specified for this host and
+ we're running a new enough neon implementation, we use
+ neon's session proxy autodetection (via libproxy). */
+ne_session_system_proxy(sess, 0);
+ne_session_system_proxy(sess2, 0);
+ }
+#endif
+
if (!timeout)
timeout = DEFAULT_HTTP_TIMEOUT;
ne_set_read_timeout(sess, timeout);
Index: subversion/libsvn_ra_neon/session.c
===
--- subversion/libsvn_ra_neon/session.c.orig
+++ subversion/libsvn_ra_neon/session.c
@@ -885,6 +885,17 @@ svn_ra_neon__open(svn_ra_session_t *sess
}
#endif
}
+#ifdef SVN_NEON_0_29
+else
+ {
+/* If we do not have any proxy specified for this host and
+ we're running a new enough neon implementation, we use
+ neon's session proxy autodetection (via libproxy). */
+ne_session_system_proxy(sess, 0);
+ne_session_system_proxy(sess2, 0);
+ }
+#endif
+
if (!timeout)
timeout = DEFAULT_HTTP_TIMEOUT;
Index: build/ac-macros/neon.m4
===
--- build/ac-macros/neon.m4.orig
+++ build/ac-macros/neon.m4
@@ -69,6 +69,11 @@ AC_DEFUN(SVN_LIB_NEON,
[Define to 1 if you have Neon 0.28 or later.])
fi
+ if test -n [`echo $NEON_VERSION | grep '^0\.29\.'`] ; then
+AC_DEFINE_UNQUOTED([SVN_NEON_0_29], [1],
+ [Define to 1 if you have Neon 0.29 or later.])
+ fi
+
for svn_allowed_neon in $NEON_ALLOWED_LIST; do
if test -n `echo $NEON_VERSION | grep ^$svn_allowed_neon` ||
test $svn_allowed_neon = any; then
@@ -160,6 +165,11 @@ AC_DEFUN(SVN_NEON_CONFIG,
[Define to 1 if you have Neon 0.28 or later.])
fi
+ if test -n [`echo $NEON_VERSION | grep '^0\.29\.'`] ; then
+AC_DEFINE_UNQUOTED([SVN_NEON_0_29], [1],
+ [Define to 1 if you have Neon 0.29 or later.])
+ fi
+
for svn_allowed_neon in $NEON_ALLOWED_LIST; do
if test -n `echo $NEON_VERSION | grep ^$svn_allowed_neon` ||
test $svn_allowed_neon = any; then
Re: [RFC] Preprocessor flags to target code to a specific API version
On Sun, Sep 26, 2010 at 5:31 PM, Dani Church dchu...@cheri.shyou.org wrote: On Sun, 26 Sep 2010, Daniel Shahaf wrote: Adding the SVN_MAYBE_POISON(svn_wc_some_function) can be done mechanically (by a script that reads the @deprecated comments and transforms the header files accordingly) --- instead of editing all headers now, we could provide a knob for people to run this script during 'make install'. I toyed around with that kind of idea, but, having just done a lot of partially-automated text processing on these files, I know just how hard it is to get it right-- I wouldn't want to leave that to an automated script. Also, I'm not a big fan of having to mangle the header files before they're usable-- I'd rather be able to point my GCC include path straight at my working copy. We already have a header-file parser: doxygen. It shouldn't be too much trouble[1] to get the xml-ified doxygen output for a given header and search for deprecated APIs based upon that. Whether or not we *should* do that is another question, but I'm just pointing out that it's technically feasible. -Hyrum [1] Famous last words
RE: [PATCH] Use neon's system proxy detection if not explicitly specified
On Mon, 2010-09-27 at 12:33 +0100, Jon Foster wrote: Hi, + if test -n [`echo $NEON_VERSION | grep '^0\.29\.'`] ; then + AC_DEFINE_UNQUOTED([SVN_NEON_0_29], [1], +[Define to 1 if you have Neon 0.29 or later.]) + fi The comment says 0.29 or later (which I think is right), but the test looks like it's checking for Neon 0.29 exactly. Will this go wrong with Neon 0.30 or 1.00? Hi, That's the patch for 1.6.12 as I remember. It is rather unclear if neon 0.30 is going to change API or not to me. I would assume not. This is also the reason why the trunk based patch contains: + if test -n [`echo $NEON_VERSION | $EGREP '^0\.(29| 3[0-9])\.'`] ; then +AC_DEFINE_UNQUOTED([SVN_NEON_0_29], [1], + [Define to 1 if you have Neon 0.29 or later.]) + fi + This should trigger at least 0.3x as valid as well. (the 1.6.12 patch is more for reference, as that is the tested one). Dominique
RE: [PATCH] Use neon's system proxy detection if not explicitly specified
Dominique Leuenberger wrote: On Mon, 2010-09-27 at 12:33 +0100, Jon Foster wrote: Hi, + if test -n [`echo $NEON_VERSION | grep '^0\.29\.'`] ; then + AC_DEFINE_UNQUOTED([SVN_NEON_0_29], [1], +[Define to 1 if you have Neon 0.29 or later.]) + fi The comment says 0.29 or later (which I think is right), but the test looks like it's checking for Neon 0.29 exactly. Will this go wrong with Neon 0.30 or 1.00? Hi, That's the patch for 1.6.12 as I remember. Yes. I hadn't realised they did this test differently, sorry. It is rather unclear if neon 0.30 is going to change API or not to me. I would assume not. If it changes the API, the build might break whatever you do. So it's probably best to assume that it doesn't change API. (Also: API changes usually cause the build to explode; this test failing will just quietly disable a feature). This is also the reason why the trunk based patch contains: + if test -n [`echo $NEON_VERSION | $EGREP '^0\.(29| 3[0-9])\.'`] ; then +AC_DEFINE_UNQUOTED([SVN_NEON_0_29], [1], + [Define to 1 if you have Neon 0.29 or later.]) + fi + This should trigger at least 0.3x as valid as well. (the 1.6.12 patch is more for reference, as that is the tested one). Better, but it'll still go wrong with Neon 0.40 or 1.00. I guess it needs to be something like: if test -n [`echo $NEON_VERSION | $EGREP '^(([1-9][0-9]*)|(0\.(29|[3-9][0-9])))\.'`] ; then ? That should match 0.29-0.99 and 1.0 or later. I'm assuming there won't ever be a 0.100 release. (Aside: This would be much easier if neon #defined a macro like NE_VERSION_NUMBER, which we could just test in an #ifdef. Unfortunately, it doesn't seem to have one). Kind regards, Jon ** This email and its attachments may be confidential and are intended solely for the use of the individual to whom it is addressed. Any views or opinions expressed are solely those of the author and do not necessarily represent those of Cabot Communications Ltd. If you are not the intended recipient of this email and its attachments, you must take no action based upon them, nor must you copy or show them to anyone. Cabot Communications Limited Verona House, Filwood Road, Bristol BS16 3RY, UK +44 (0) 1179584232 Co. Registered in England number 02817269 Please contact the sender if you believe you have received this email in error. ** __ This email has been scanned by the MessageLabs Email Security System. For more information please visit http://www.messagelabs.com/email __
Build tools/ by default? (was: Re: place of svnrdump)
On Sat, Sep 25, 2010 at 04:40:01PM +0200, Daniel Shahaf wrote: Ramkumar Ramachandra wrote on Sat, Sep 25, 2010 at 19:15:16 +0530: I have no interest in politics, and I don't want to speculate why svnmucc isn't built by `make` by default. Because it lives in tools/. What about we start building tools by default? Not install, just build? This would help us catch compile-breakage in tools more quickly. Stefan Index: configure.ac === --- configure.ac(revision 1000128) +++ configure.ac(working copy) @@ -702,7 +702,7 @@ dnl Build and install rules --- INSTALL_STATIC_RULES=install-bin install-docs INSTALL_RULES=install-fsmod-lib install-ramod-lib install-lib install-include install-static INSTALL_RULES=$INSTALL_RULES $INSTALL_APACHE_RULE -BUILD_RULES=fsmod-lib ramod-lib lib bin test $BUILD_APACHE_RULE +BUILD_RULES=fsmod-lib ramod-lib lib bin test $BUILD_APACHE_RULE tools if test $svn_lib_berkeley_db = yes; then BUILD_RULES=$BUILD_RULES bdb-lib bdb-test
RE: [PATCH] Use neon's system proxy detection if not explicitly specified
On Mon, 2010-09-27 at 13:19 +0100, Jon Foster wrote: Better, but it'll still go wrong with Neon 0.40 or 1.00. I guess it needs to be something like: if test -n [`echo $NEON_VERSION | $EGREP '^(([1-9][0-9]*)|(0\.(29|[3-9][0-9])))\.'`] ; then ? That should match 0.29-0.99 and 1.0 or later. I'm assuming there won't ever be a 0.100 release. Right; the up to 0.39 check is a lazy copy paste from all the other check in neon.m4 :) The check which exists already for 0.28 is like this: if test -n [`echo $NEON_VERSION | $EGREP '^0\.(2[8-9]| 3[0-9])\.'`] ; then AC_DEFINE_UNQUOTED([SVN_NEON_0_28], [1], [Define to 1 if you have Neon 0.28 or later.]) fi For me either way is fine: I can update the patch to also detect newer versions as suggest by you. Which in turn will still break all the other detections of SVN_NEON_0_28 and older. Or we keep them 'in sync' together and fix them all together at a later stage. (Aside: This would be much easier if neon #defined a macro like NE_VERSION_NUMBER, which we could just test in an #ifdef. Unfortunately, it doesn't seem to have one). Indeed. This was actually what took me most of the time to find out how the heck do you guys know what version neon has :) until I found neon.m4. Dominique
Re: place of svnrdump (was: Re: svnmucc multiline property issue)
On Sat, Sep 25, 2010 at 02:43:37PM +0200, Daniel Shahaf wrote: Ramkumar Ramachandra wrote on Sat, Sep 25, 2010 at 11:59:49 +0530: Daniel Shahaf writes: Would svnrdump benefit if, once 1.7.x branched and RC's start being rolled, it were subjected to a more relaxed backporting policy? If so, we might consider moving it to tools/ for 1.7.x, with intent to move it back to subversion/svnrdump/ for 1.8.x (as soon as 1.7.x is branched from trunk). Hm? I don't understand how it'll help backporting. I already maintain an out-of-tree build that successfully compiles against 1.6 [1]. It'll be fairly trivial to write an svn18_compat module. We'll be able to follow a more relaxed Is this change backportable policy. I don't really mind where svnrdump lives. The community is committed to supporting both the tools/ and subversion/ directories. If backwards-compat rules automatically apply to everything below subversion/ (they do?), and that people feel that svnrdump may still change in backwards-incompatible ways, we might as well decide to make the subversion/svnrdump directory exempt from such guarantees during the 1.7 release. It is a simple technical decision: Do we think that the current feature set is worth supporting under our backwards-compat rules? I do. Hyrum K. Wright writes: I would add the further thought that as this is a tool rather than a first-class component, I think we can play a little bit looser with the rules. For what it's worth, I have a different opinion on this issue. It really doesn't matter. It's just the name of a directory and a set of promises we give to our users about backwards-compat. There's no need for hard feelings. Stefan
NODES table - conditional compilation for op_depth
Philip and I have started implementing op_depth in the NODES table, but we soon found there is more to do than simply calculating a value here and there. (Implementing op_depth means enabling multiply nested copies/replacements/adds/deletes etc., with the ability to revert at each level.) In the meantime, some tests were breaking, so we have made the full op_depth support conditional on SVN_WC__OP_DEPTH. Why? The interim 0/1/2 op_depth values have been working OK in the limited context of the amount of nesting that BASE+WORKING supports. We might want to make a transition from BASE_NODE/WORKING_NODE to NODES first, before enabling the full op_depth support. That is probably the main reason why this further conditional is useful. The alternative would be to complete op_depth support before switching the default build over to NODES. Any concerns about working within SVN_WC__OP_DEPTH? - Julian
Re: place of svnrdump
On 2010-09-27 09:45, Ramkumar Ramachandra wrote: ... I just benchmarked it recently and found that it dumps 1 revisions of the ASF repository in 106 seconds: that's about 94 revisions per second. Wow! That's magnitudes better than the 5 to 10 revisions per second I'm used to (I think that's using svnsync). While we're at it... svnsync's slowness is particularly painful when doing 'svnsync copy-revprops'. With revprop changes enabled, any revprops may be changed at any time. So to maintain an up-to-date mirror, one would like to copy *all* revprops at the very least once per day. With a repos of average corporate size, though, that can take the whole night and soon longer than the developers need to go home and come back to work next morning (to find the mirror lagging). So one could copy only the youngest 1000 revprops each night and do a complete run every weekend. Or script a revprop-change hook that propagates revprop change signals to mirrors. :( svnrdump won't help in that compartment, would it? Thanks, ~Neels signature.asc Description: OpenPGP digital signature
Re: place of svnrdump (was: Re: svnmucc multiline property issue)
Hi Stefan, Stefan Sperling writes: I don't really mind where svnrdump lives. The community is committed to supporting both the tools/ and subversion/ directories. tools and subversion are merely directory names. All I'm saying is this: I don't want packaging/ distribution overheads for such a simple package; users should be able to use whatever Subversion-interop tools that other developers build by just having Subversion installed. If backwards-compat rules automatically apply to everything below subversion/ (they do?), and that people feel that svnrdump may still change in backwards-incompatible ways, we might as well decide to make the subversion/svnrdump directory exempt from such guarantees during the 1.7 release. It is a simple technical decision: Do we think that the current feature set is worth supporting under our backwards-compat rules? I do. Hm, I still don't understand this back-porting thing very well. Does it mean that the svnrdump should always do what it currently does functionally (plus anything additional)? Does it mean that any improvements to the command-line UI should ensure that the current command-line UI still works? Does it mean that the public API it exposes through the headers should not break- do we instead have to write corresponding '_2' functions? It seems to be quite sane at the moment, and I don't think backporting is an issue; I'm not very experienced in this though, so I wouldn't take my own opinion too seriously. Hyrum K. Wright writes: I would add the further thought that as this is a tool rather than a first-class component, I think we can play a little bit looser with the rules. For what it's worth, I have a different opinion on this issue. It really doesn't matter. It's just the name of a directory and a set of promises we give to our users about backwards-compat. There's no need for hard feelings. Hey, no hard feelings! I was merely citing how other version control systems make it easy for users to get access to their own history, and suggesting that Subversion should too. In the long-term, I think of svnrdump as just a simple dumping/ loading functionality of Subversion: I don't really care *how* it's present; I just think it should be present: either as part of svnrdump, svnadmin, svnsync, or something else. -- Ram
Re: place of svnrdump (was: Re: svnmucc multiline property issue)
On Mon, Sep 27, 2010 at 07:48:06PM +0530, Ramkumar Ramachandra wrote: Hi Stefan, Stefan Sperling writes: I don't really mind where svnrdump lives. The community is committed to supporting both the tools/ and subversion/ directories. tools and subversion are merely directory names. All I'm saying is this: I don't want packaging/ distribution overheads for such a simple package; users should be able to use whatever Subversion-interop tools that other developers build by just having Subversion installed. There are many interoperability tools that are built on top of Subversion, and they're hosted as independent projects. By the above logic, we'd have to ship all those, and host them in our repository. And what does having Subversion installed really mean? E.g. in the Linux world, the Subversion client/server packages are often separate, but not always. It's also possible for svnsync to live in a separate package from the svn binary. And if you install from source, you get whatever the make install target installs. And maybe you also run make install-tools? Who knows. The point is that this is something packagers need to worry about, not us. With a well-maintained distribution, you can also install svnmucc easily, just like you can install svn easily. Of course, svnrdump is more likely to end up being installed if it gets installed with the default make install target. But packagers might as well decide to split the result of make install into separate packages, and we can't do anything about it. In practice, I don't think any of this is very important. People who need the svnrdump tool will find it no matter what. Even if it was hosted as an entirely separate project. If backwards-compat rules automatically apply to everything below subversion/ (they do?), and that people feel that svnrdump may still change in backwards-incompatible ways, we might as well decide to make the subversion/svnrdump directory exempt from such guarantees during the 1.7 release. It is a simple technical decision: Do we think that the current feature set is worth supporting under our backwards-compat rules? I do. Hm, I still don't understand this back-porting thing very well. Does it mean that the svnrdump should always do what it currently does functionally (plus anything additional)? Does it mean that any improvements to the command-line UI should ensure that the current command-line UI still works? Does it mean that the public API it exposes through the headers should not break- do we instead have to write corresponding '_2' functions? It means all of the above. We'll promise to support its current state until Subversion 2.0 breaks the world. That's why it's important to make sure everyone agrees that it is ready for that level of dedication. If it isn't, then we need to make sure our users understand that (by moving it to tools/, or by declaring it as experimental, or whatever). Hey, no hard feelings! I was merely citing how other version control systems make it easy for users to get access to their own history, There are quite a few systems that make getting at history really hard. But people only realise that when they're trying to migrate away to something else :) and suggesting that Subversion should too. Certainly! In the long-term, I think of svnrdump as just a simple dumping/ loading functionality of Subversion: I don't really care *how* it's present; I just think it should be present: either as part of svnrdump, svnadmin, svnsync, or something else. Yes, it's a valuable feature to have. Stefan
Re: place of svnrdump (was: Re: svnmucc multiline property issue)
Hi Stefan, Stefan Sperling writes: tools and subversion are merely directory names. All I'm saying is this: I don't want packaging/ distribution overheads for such a simple package; users should be able to use whatever Subversion-interop tools that other developers build by just having Subversion installed. There are many interoperability tools that are built on top of Subversion, and they're hosted as independent projects. By the above logic, we'd have to ship all those, and host them in our repository. No. That's what I tried to point out in my email: the interop software are tools like fast-import for hg and bzr, or even git-p4. That's not what svnrdump is: svnrdump itself is *far* from being able to provide interop. It's the *infrastructure* that's necessary for interop tools to be built in a sane and maintainable manner. If you're interested, check out git.git contrib/svn-fe leveraging the infrastructure in vcs-svn/ to convert a Subverion dumpfile v2 into a fast-import stream. It's VERY non-trivial. And what does having Subversion installed really mean? E.g. in the Linux world, the Subversion client/server packages are often separate, but not always. It's also possible for svnsync to live in a separate package from the svn binary. And if you install from source, you get whatever the make install target installs. And maybe you also run make install-tools? Who knows. The point is that this is something packagers need to worry about, not us. With a well-maintained distribution, you can also install svnmucc easily, just like you can install svn easily. Of course, svnrdump is more likely to end up being installed if it gets installed with the default make install target. But packagers might as well decide to split the result of make install into separate packages, and we can't do anything about it. In practice, I don't think any of this is very important. People who need the svnrdump tool will find it no matter what. Even if it was hosted as an entirely separate project. I see- that's an interesting perspective. Hm, I still don't understand this back-porting thing very well. Does it mean that the svnrdump should always do what it currently does functionally (plus anything additional)? Does it mean that any improvements to the command-line UI should ensure that the current command-line UI still works? Does it mean that the public API it exposes through the headers should not break- do we instead have to write corresponding '_2' functions? It means all of the above. We'll promise to support its current state until Subversion 2.0 breaks the world. That's why it's important to make sure everyone agrees that it is ready for that level of dedication. If it isn't, then we need to make sure our users understand that (by moving it to tools/, or by declaring it as experimental, or whatever). Ah. Yeah, I think it's sane enough for this. I'll put in as much work as I can before the release to fix perf issues. For now, it passes the complete svnsync testsuite but for Issue #3717. -- Ram
Re: buildbot failure in ASF Buildbot on svn-x64-centos gcc
Are these authz failures expected? On Mon, Sep 27, 2010 at 12:44, build...@apache.org wrote: The Buildbot has detected a new failure of svn-x64-centos gcc on ASF Buildbot. Full details are available at: http://ci.apache.org/builders/svn-x64-centos%20gcc/builds/1343 Buildbot URL: http://ci.apache.org/ Buildslave for this Build: svn-x64-centos Build Reason: Build Source Stamp: [branch subversion/branches/1.6.x] 1001802 Blamelist: cmpilato,hwright,stsp BUILD FAILED: failed Test fsfs+ra_neon sincerely, -The Buildbot
Re: migration to NODES
On Fri, 2010-09-24, Greg Stein wrote: On Fri, Sep 24, 2010 at 11:16, Julian Foad julian.f...@wandisco.com wrote: ... I think we should produce a test framework that can give us a WC containing all the different possible WC states. Then we can write tests against this framework, some tests that test specific state, and other tests that apply the same operations to all the states and check that it works in all the states that it should. This requires a manual process of thinking of all states and all permutations. I don't trust it. This kind of testing is more about checking that the design is implemented and that the code paths are exercised ... If we could somehow capture working copies from during normal test runs, *then* I think we'd have everything. We can easily get the terminal state for each test, which is a great first step. It would be great if we could also get the intermediate steps. ... while this kind is more about checking for regression or unwanted changes of behaviour. The two approaches are complementary. Haven't thought much about it, but my thoughts were somewhere along that line. I have been thinking this for a while. As yet I've just got a rudimentary constructor for a WC containing (nearly) all *base* states. Not changes, yet. Here it is: Not sure if it solves everything, but it is a good start. Review below: Thanks for the review comments. All useful. - Julian
Re: svn commit: r1001677 - /subversion/trunk/notes/wc-ng/nodes
NICE!! On Mon, Sep 27, 2010 at 07:40, e...@apache.org wrote: Author: ehu Date: Mon Sep 27 11:40:18 2010 New Revision: 1001677 URL: http://svn.apache.org/viewvc?rev=1001677view=rev Log: Add NODES design considerations document in nodes/wc-ng/nodes. Added: subversion/trunk/notes/wc-ng/nodes Added: subversion/trunk/notes/wc-ng/nodes URL: http://svn.apache.org/viewvc/subversion/trunk/notes/wc-ng/nodes?rev=1001677view=auto == --- subversion/trunk/notes/wc-ng/nodes (added) +++ subversion/trunk/notes/wc-ng/nodes Mon Sep 27 11:40:18 2010 @@ -0,0 +1,159 @@ + +Description of the NODES table +== + + + * Introduction + * Inclusion of BASE nodes + * Rows to store state + * Ordering rows into layers + * Visibility of multiple op_depth rows + * Restructuring the tree means adding rows + * + + +Introduction + + +The entire original design of wc-ng evolves around the notion that +there are a number of states in a working copy, each of which needs +to be managed. All operations - excluding merge - operate on three +trees: BASE, WORKING and ACTUAL. + +For an in-depth description of what each means, the reader is referred +to other documentation, also in the notes/ directory. In short, BASE +is what was checked out from the repository; WORKING includes +modifications mode with Subversion commands while ACTUAL also includes +changes which have been made with non-Subversion aware tools (rm, cp, etc.). + +The idea that there are three trees works - mostly. There is no need +for more trees outside the area of the metadata administration and even +then three trees got us pretty far. The problem starts when one realizes +tree modifications can be overlapping or layered. Imagine a tree with +a replaced subtree. It's possible to replace a subtree within the +replacement. Imagine that happened and that the user wants to revert +one of the replacements. Given a 'flat' system, with just enough columns +in the database to record the 'old' and 'new' information per node, a single +revert can be supported. However, in the example with the double +replacement above, that would mean it's impossible to revert one of the +two replacements: either there's not enough information in the deepest +replacement to execute the highest level replacement or vice versa +- depending on which information was selected to be stored in the new +columns. + +The NODES table is the answer to this problem: instead of having a single +row it a table with WORKING nodes with just enough columns to record +(as per the example) a replacement, the solution is to record different +states by having multiple rows. + + + +Inclusion of BASE nodes +--- + +The original technical design of wc-ng included a WORKING_NODE and a +BASE_NODE table. As described in the introduction, the WORKING_NODE +table was replaced with NODES. However, the BASE_NODE table stores +roughly the same state information that WORKING_NODE did. Additionally, +in a number of situations, the system isn't interested in the type of +state it gets returned (BASE or WORKING) - it just wants the latest. + +As a result the BASE_NODE table has been integrated into the NODES +table. + +The main difference between the WORKING_NODE and BASE_NODE tables was +that the BASE_NODE table contained a few caching fields which are +not relevant to WORKING_NODE. Moving those to a separate table was +determined to be wasteful because the primary key of that table +whould be much larger than any information stored in it in the first +place. + + + +Rows to store state +--- + +Rows of the NODES table store state of nodes in the BASE tree +and the layers in the WORKING tree. Note that these nodes do not +need to exist in the working copy presented to the user: they may +be 'absent', 'not-present' or just removed (rm) without using +Subversion commands. + +A row contains information linking to the repository, if the node +was received from a repository. This reference may be a link to +the original nodes for copied or moved nodes, but for rows designating +BASE state, they refer to the repository location which was checked +out from. + +Additionally, the rows contain information about local modifications +such copy, move or delete operations. + + + +Ordering rows into layers +- + +Since the table might contain more than one row per (wc_id, local_relpath) +combination, an ordering mechanism needs to be added. To that effect +the 'op_depth' value has been devised. The op_depth is an integer +indicating the depth of the operation which modified the tree in order +for the node to enter the state indicated in the row. + +Every row for the (wc_id, local_relpath) combination must have a unique +op_depth associated with it.
Re: Bindings for svn_auth_get_platform_specific_client_providers missing
Hey Folks, I've attached an updated patch with a test case. The testcase is a bity ugly, since it tests each element of the returned array and keeps the result in its own variable. This is needed since I can't predict how many elements there will be in the returned array, but I didn't want to change the test case too invasively (to use the done_testing function, for example). Any chance someone could have a look at this simple patch and perhaps commit it? Gr. Matthijs signature.asc Description: Digital signature
Re: svnrdump tool
Hi Daniel, Daniel Shahaf writes: svnsync allows you to sync a subdir of a repository (i.e., svnsync $REPOS/trunk/A/B $MIRROR ), but does it also create /trunk/A/B in the mirror? Yes, it does. But for now I still think that svnrdump invocation I quoted above shouldn't have outputted a 'create /trunk' entry in the dumpfile :-). @all, what do you think? Again, it works exactly like svnsync- you might like to read the tests. Here's a verbose example. $ cat /test.dump Node-path: test.txt Node-path: trunk Node-path: trunk/A Node-path: trunk/A/B Node-path: trunk/A/B/E Node-path: trunk/A/B/E/alpha Node-path: trunk/A/B/E/beta Node-path: trunk/A/B/F Node-path: trunk/A/B/lambda Node-path: trunk/A/C Node-path: trunk/A/D Node-path: trunk/A/D/G Node-path: trunk/A/D/G/pi Node-path: trunk/A/D/G/rho Node-path: trunk/A/D/G/tau Node-path: trunk/A/D/H Node-path: trunk/A/D/H/chi Node-path: trunk/A/D/H/omega Node-path: trunk/A/D/H/psi Node-path: trunk/A/D/gamma Node-path: trunk/A/mu Node-path: trunk/iota Node-path: trunk/A/D/H/psi Node-path: trunk/A Node-path: trunk/iota Node-path: trunk/B Node-path: trunk/A Node-path: trunk/A Node-path: trunk/B/D Node-path: trunk $ # svnadmin create /test-repo, /mirror and enable pre-revprop-change $ svnadmin load /test-repo /test.dump $ svnsync init file:///mirror file:///test-repo/trunk/A/B $ svnsync sync file:///mirror $ svnadmin dump /mirror | grep Node-path: Node-path: trunk Node-path: trunk/A Node-path: trunk/A/B Node-path: trunk/A/B/E Node-path: trunk/A/B/E/alpha Node-path: trunk/A/B/E/beta Node-path: trunk/A/B/F Node-path: trunk/A/B/lambda Node-path: trunk/A Node-path: trunk/A Node-path: trunk/A Node-path: trunk/A/G Node-path: trunk/A/G/pi Node-path: trunk/A/G/rho Node-path: trunk/A/G/tau Node-path: trunk/A/H Node-path: trunk/A/H/chi Node-path: trunk/A/H/omega Node-path: trunk/A/H/psi Node-path: trunk/A/gamma Node-path: trunk $ svnrdump dump file:///test-repo/trunk/A/B Node-path: trunk Node-path: trunk/A Node-path: trunk/A/B Node-path: trunk/A/B/E Node-path: trunk/A/B/E/alpha Node-path: trunk/A/B/E/beta Node-path: trunk/A/B/F Node-path: trunk/A/B/lambda Node-path: trunk/A Node-path: trunk/A Node-path: trunk/A Node-path: trunk/A/G Node-path: trunk/A/G/pi Node-path: trunk/A/G/rho Node-path: trunk/A/G/tau Node-path: trunk/A/H Node-path: trunk/A/H/chi Node-path: trunk/A/H/omega Node-path: trunk/A/H/psi Node-path: trunk/A/gamma Node-path: trunk Hope that helps. -- Ram
Some tips on profiling
Hi Stefan, Could you tell me which tools you use to profile the various applications in trunk? I'm looking to profile svnrdump to fix some perf issues, but OProfile doesn't seem to work for me. Thanks. -- Ram
[WIP] Gracefully handle svn_wc_prop_set4() errors in svn patch
Hi!
Questions
-
* Is it sane to add a svn_wc_notify_failed_prop_patch action for this
special case? We're starting to have a lot of actions.
* What is wrong with the way I handle the error? I hit err_abort() when
the program terminates. (I'm expecting the answer to hurt a bit - this
is surely something that I should have understood by now). I thought
that since the error is allocated on the heap I could just store the
pointer to it and free it later, e.g. call svn_error_clear().
[[[
Print a warning instead of error out if a property could not be set on a
path with 'svn patch'.
* subversion/svn/notify.c
(notify): Add svn_wc_notify_failed_prop_patch case.
* subversion/include/svn_wc.h
(svn_wc_notify_action_t): Add svn_wc_notify_failed_prop_patch field.
(svn_wc_notify_t): Update doc string for 'err' field to mention that
it is set for svn_wc_notify_failed_prop_patch.
* subversion/libsvn_client/patch.c
(prop_patch_target_t): Add 'was_rejected' and 'err' field to record
failed property patches.
(send_patch_notification): Send svn_wc_notify_failed_prop_patch
notifications.
(install_patched_prop_targets): Record failed propsets.
]]]
Daniel
Index: subversion/svn/notify.c
===
--- subversion/svn/notify.c (revision 1001829)
+++ subversion/svn/notify.c (arbetskopia)
@@ -464,6 +464,16 @@ notify(void *baton, const svn_wc_notify_t *n, apr_
goto print_error;
break;
+case svn_wc_notify_failed_prop_patch:
+ nb-received_some_change = TRUE;
+ err = svn_cmdline_printf(pool,
+ _(property '%s' rejected from '%s'.\n),
+ n-prop_name, path_local);
+ svn_handle_warning2(stderr, n-err, svn: );
+ if (err)
+goto print_error;
+ break;
+
case svn_wc_notify_update_update:
case svn_wc_notify_merge_record_info:
{
Index: subversion/include/svn_wc.h
===
--- subversion/include/svn_wc.h (revision 1001829)
+++ subversion/include/svn_wc.h (arbetskopia)
@@ -1089,8 +1089,11 @@ typedef enum svn_wc_notify_action_t
/** The server has instructed the client to follow a URL
* redirection.
* @since New in 1.7. */
- svn_wc_notify_url_redirect
+ svn_wc_notify_url_redirect,
+ /** A hunk from a patch could not be applied. */
+ svn_wc_notify_failed_prop_patch
+
} svn_wc_notify_action_t;
@@ -1198,7 +1201,8 @@ typedef struct svn_wc_notify_t {
/** Points to an error describing the reason for the failure when @c
* action is one of the following: #svn_wc_notify_failed_lock,
- * #svn_wc_notify_failed_unlock, #svn_wc_notify_failed_external.
+ * #svn_wc_notify_failed_unlock, #svn_wc_notify_failed_external,
+ * #svn_wc_notify_failed_prop_patch.
* Is @c NULL otherwise. */
svn_error_t *err;
Index: subversion/libsvn_client/patch.c
===
--- subversion/libsvn_client/patch.c(revision 1001829)
+++ subversion/libsvn_client/patch.c(arbetskopia)
@@ -130,6 +130,12 @@ typedef struct prop_patch_target_t {
/* ### Here we'll add flags telling if the prop was added, deleted,
* ### had_rejects, had_local_mods prior to patching and so on. */
+
+ /* TRUE if the property could not be set on the path. */
+ svn_boolean_t was_rejected;
+
+ /* Set if was_rejected is TRUE. */
+ svn_error_t *err;
} prop_patch_target_t;
typedef struct patch_target_t {
@@ -1573,6 +1579,22 @@ send_patch_notification(const patch_target_t *targ
prop_target = svn__apr_hash_index_val(hash_index);
+ if (prop_target-was_rejected)
+{
+ svn_wc_notify_t *notify;
+ svn_wc_notify_action_t action = svn_wc_notify_failed_prop_patch;
+
+ notify = svn_wc_create_notify(target-local_abspath
+? target-local_abspath
+: target-local_relpath,
+action, pool);
+ notify-prop_name = prop_target-name;
+ notify-err = prop_target-err;
+
+ (*ctx-notify_func2)(ctx-notify_baton2, notify, pool);
+ svn_error_clear(prop_target-err);
+}
+
for (i = 0; i prop_target-content_info-hunks-nelts; i++)
{
const hunk_info_t *hi;
@@ -2189,6 +2211,7 @@ install_patched_prop_targets(patch_target_t *targe
svn_stringbuf_t *prop_content;
const char *eol_str;
svn_boolean_t eof;
+ svn_error_t *err;
svn_pool_clear(iterpool);
@@ -2260,14 +2283,23 @@ install_patched_prop_targets(patch_target_t *targe
* ### stsp: I'd say reject the property hunk.
* ### We should verify all modified prop hunk texts using
* ###
Re: Some tips on profiling
On 27.09.2010 21:44, Ramkumar Ramachandra wrote: Hi Stefan, Could you tell me which tools you use to profile the various applications in trunk? I'm looking to profile svnrdump to fix some perf issues, but OProfile doesn't seem to work for me. Thanks. -- Ram Hi Ram, Under Linux, I'm using Valgrind / Callgrind and visualize in KCachegrind. That gives me a good idea of what code gets executed too often, how often a jump (loop or condition) has been taken etc. It will not show the non-user and non-CPU runtime, e.g. wait for disk I/O. And it will slow the execution be a factor of 100 (YMMV). Under Windows, I'm using the VisualStudio sampling profiler. The measurements are pretty accurate and the overhead is low. It does not tell me, how often a certain code path has been executed. Due to the low overhead, it is well-suited for long running (1 min) operations. Also, I find a simple time command very useful to get a first impression whether my code is bound by user-runtime, I/O wait or system runtime. To collect data on I/O activity, compression rates and other not readily available information, I use simple static counters and printf() their values at the end of each client call, for instance. Hope that helps. -- Stefan^2.
Re: svn commit: r1001413 - in /subversion/branches/performance/subversion: include/svn_string.h libsvn_subr/svn_string.c
+1 to merge to trunk.
On Sun, Sep 26, 2010 at 6:01 AM, stef...@apache.org wrote:
Author: stefan2
Date: Sun Sep 26 11:01:03 2010
New Revision: 1001413
URL: http://svn.apache.org/viewvc?rev=1001413view=rev
Log:
Extensively document the benefits of svn_stringbuf_appendbyte and
the rationals behind its implementation. To simplify the explanations,
one statement had to be moved.
* subversion/include/svn_string.h
(svn_stringbuf_appendbyte): extend docstring to indicate that this method
is cheaper to call and execute
* subversion/libsvn_subr/svn_string.c
(svn_stringbuf_appendbyte): reorder statements for easier description;
add extensive description about the optimizations done
Modified:
subversion/branches/performance/subversion/include/svn_string.h
subversion/branches/performance/subversion/libsvn_subr/svn_string.c
Modified: subversion/branches/performance/subversion/include/svn_string.h
URL:
http://svn.apache.org/viewvc/subversion/branches/performance/subversion/include/svn_string.h?rev=1001413r1=1001412r2=1001413view=diff
==
--- subversion/branches/performance/subversion/include/svn_string.h (original)
+++ subversion/branches/performance/subversion/include/svn_string.h Sun Sep
26 11:01:03 2010
@@ -259,6 +259,10 @@ void
svn_stringbuf_fillchar(svn_stringbuf_t *str, unsigned char c);
/** Append a single character @a byte onto @a targetstr.
+ * This is an optimized version of @ref svn_stringbuf_appendbytes
+ * that is much faster to call and execute. Gains vary with the ABI.
+ * The advantages extend beyond the actual call because the reduced
+ * register pressure allows for more optimization within the caller.
*
* reallocs if necessary. @a targetstr is affected, nothing else is.
* @since New in 1.7.
Modified: subversion/branches/performance/subversion/libsvn_subr/svn_string.c
URL:
http://svn.apache.org/viewvc/subversion/branches/performance/subversion/libsvn_subr/svn_string.c?rev=1001413r1=1001412r2=1001413view=diff
==
--- subversion/branches/performance/subversion/libsvn_subr/svn_string.c
(original)
+++ subversion/branches/performance/subversion/libsvn_subr/svn_string.c Sun
Sep 26 11:01:03 2010
@@ -393,28 +393,60 @@ svn_stringbuf_ensure(svn_stringbuf_t *st
}
+/* WARNING - Optimized code ahead!
+ * This function has been hand-tuned for performance. Please read
+ * the comments below before modifying the code.
+ */
void
svn_stringbuf_appendbyte(svn_stringbuf_t *str, char byte)
{
+ char *dest;
+ apr_size_t old_len = str-len;
+
/* 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.
*/
- apr_size_t old_len = str-len;
- if (str-blocksize old_len + 1)
+ if (str-blocksize old_len + 1)
{
- char *dest = str-data;
+ /* The following read does not depend this write, so we
+ * can issue the write first to minimize register pressure:
+ * The value of old_len+1 is no longer needed; on most processors,
+ * dest[old_len+1] will be calculated implicitly as part of
+ * the addressing scheme.
+ */
+ str-len = old_len+1;
+
+ /* Since the compiler cannot be sure that *src-data and *src
+ * don't overlap, we read src-data *once* before writing
+ * to *src-data. Replacing dest with str-data would force
+ * the compiler to read it again after the first byte.
+ */
+ dest = str-data;
+ /* If not already available in a register as per ABI, load
+ * byte into the register (e.g. the one freed from old_len+1),
+ * then write it to the string buffer and terminate it properly.
+ *
+ * Including the byte fetch, all operations so far could be
+ * issued at once and be scheduled at the CPU's descression.
+ * Most likely, no-one will soon depend on the data that will be
+ * written in this function. So, no stalls there, either.
+ */
dest[old_len] = byte;
dest[old_len+1] = '\0';
-
- str-len = old_len+1;
}
else
{
/* we need to re-allocate the string buffer
* - let the more generic implementation take care of that part
*/
+
+ /* Depending on the ABI, byte is a register value. If we were
+ * to take its address directly, the compiler might decide to
+ * put in on the stack *unconditionally*, even if that would
+ * only be necessary for this block.
+ */
char b = byte;
svn_stringbuf_appendbytes(str, b, 1);
}
