Re: Some tips on profiling
On Thu, Sep 30, 2010 at 07:58:45AM +0530, Ramkumar Ramachandra wrote: > Stefan Fuhrmann writes: > > At least the results are much more useful when there is > > a tool like Kcachegrind that allows easy navigation though > > the huge amount of information that was gathered. > > Yep! The visualizer is quite awesome :) On a tangential note; I spotted OprofileUI [1] the other day when googling for a graphical frontend to Oprofile. I haven't tested it myself. [1] http://labs.o-hand.com/oprofileui/ Daniel
Re: Some tips on profiling
Hi Stefan, Stefan Fuhrmann writes: > My measurements seem to support what Philip wrote: > The expensive part is run on the server. Even with my > optimized server, the svnrdump CPU usage is less than > the time taken by the server. Some numbers (hot file > cache): > > svnadmin dump > 1.7 trunk 70s real 66s user 4s system > perf-branch 30s real 28s user 2s system > > 1.7 trunk svnrdump > ra-local 88s real 81s user 7s system > svn: (1.7 trunk) 99s real 6s user 4s system > svn: (perf-branch, cold) 72s real 5s user 6s system > svn: (perf-branch, hot) 17s real 5s user 5s system > > Thus, svnrdump is slower only for ra-local where it is > of no particular use in the first place. To really speed > things up, the caching infrastructure from the performance > branch should be merged into /trunk. Wow, that looks awesome. Yeah, Daniel suggested that I run svnrdump against your perf branch yesterday, but I wasn't able to get your branch to build: subversion/libsvn_subr/svn_file_handle_cache.c:890: error: 'svn_file_handle_cache_t' has no member named 'mutex' What exactly are the changes that "hot" introduces- can you point me to the specific revision numbers that we intend to merge? > >@Stefan: Thoughts on hacking APR hashtables directly? > > > Are you sure?! Which operation is the most expensive one > and how often is it called? Who calls it and why? My bad. I got muddled up when using ra_local: when I saw lots of MD5 functions, I assumed it had to do something with the hashtable since the checksum was caculated by server-side. The profiler profiled both server-side and client-side. Over svn://, I got a much cleaner output. The new results indicate: 1. Server-client IO is the major bottleneck, as Philip and you pointed out. 2. On the client side, the major bottlneck is the IO during textdelta application. David (CC'ed now) and I are trying some experiments with a single temporary file. Thoughts? > At least the results are much more useful when there is > a tool like Kcachegrind that allows easy navigation though > the huge amount of information that was gathered. Yep! The visualizer is quite awesome :) -- Ram
Re: svn commit: r1002898 - in /subversion/trunk: ./ subversion/include/svn_string.h subversion/libsvn_subr/svn_string.c
On 09/29/2010 05:01 PM, stef...@apache.org wrote: Author: stefan2 Date: Thu Sep 30 00:01:45 2010 New Revision: 1002898 URL: http://svn.apache.org/viewvc?rev=1002898&view=rev Log: Merge r1001413 from the performance branch. Improve documentation on svn_stringbuf_appendbyte. Approved by: Hyrum K. Wright /** 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. When you say ABI, you actually mean the C calling convention ABI, not the Subversion ABI? I think this would be a good distinction to document here, since Subversion has its own ABI that we strictly maintain. Blair
Re: Some tips on profiling
On 29.09.2010 10:45, Ramkumar Ramachandra wrote: Hi Philip, Philip Martin writes: The performance of svnrdump is likely to be dominated by IO from the repository, network or disk depending on the RA layer. strace is a useful tool to see opens/reads/writes. You can see what order the calls occur, how many there are, how big they are and how long they take. Ah, thanks for the tip. My measurements seem to support what Philip wrote: The expensive part is run on the server. Even with my optimized server, the svnrdump CPU usage is less than the time taken by the server. Some numbers (hot file cache): svnadmin dump 1.7 trunk 70s real 66s user 4s system perf-branch 30s real 28s user 2s system 1.7 trunk svnrdump ra-local 88s real 81s user 7s system svn: (1.7 trunk) 99s real 6s user 4s system svn: (perf-branch, cold) 72s real 5s user 6s system svn: (perf-branch, hot) 17s real 5s user 5s system Thus, svnrdump is slower only for ra-local where it is of no particular use in the first place. To really speed things up, the caching infrastructure from the performance branch should be merged into /trunk. Valgrind/Callgrind is good and doesn't require you to instrument the code, but it does help to build with debug information. It does impose a massive runtime overhead. I don't mind -- I'm mostly using some remote machines to gather the profiling data :) This is what I get when dumping 1000 revisions from a local mirror of the Subversion repository over ra_neon: CPU: Core 2, speed 1200 MHz (estimated) Counted CPU_CLK_UNHALTED events (Clock cycles when not halted) with a unit mask of 0x00 (Unhalted core cycles) count 10 samples %app name symbol name 4738 41.1893 no-vmlinux (no symbols) 1037 9.0150 libxml2.so.2.6.32(no symbols) 700 6.0854 libneon.so.27.1.2(no symbols) 238 2.0690 libc-2.7.so _int_malloc 228 1.9821 libc-2.7.so memcpy 221 1.9212 libc-2.7.so memset 217 1.8865 libc-2.7.so strlen 191 1.6604 libsvn_subr-1.so.0.0.0 decode_bytes 180 1.5648 libc-2.7.so vfprintf 171 1.4866 libc-2.7.so strcmp 153 1.3301 libapr-1.so.0.2.12 apr_hashfunc_default 134 1.1649 libapr-1.so.0.2.12 apr_vformatter 130 1.1301 libapr-1.so.0.2.12 apr_palloc That's on my Debian desktop. At the recent Apache Retreat I tried to demonstrate OProfile on my Ubuntu laptop and could not get it to work properly, probably because I forgot about -fno-omit-frame-pointer. Ah, now I see why it didn't work for me. The data from Callgrind is very interesting- it seems to suggest that APR hashtables are prohibitively expensive. @Stefan: Thoughts on hacking APR hashtables directly? Are you sure?! Which operation is the most expensive one and how often is it called? Who calls it and why? Finally there is traditional gprof. It's a long time since I used it so I don't remember the details. You instrument the code at compile time using CFLAGS=-pg. If an instrumented function foo calls into a library bar that is not instrumented then bar is invisible, all you see is how long foo took to execute. Yes, I used gprof initially. Callgrind is WAY more useful. At least the results are much more useful when there is a tool like Kcachegrind that allows easy navigation though the huge amount of information that was gathered. -- Stefan^2.
Re: svn commit: r1002839 - /subversion/site/publish/docs/release-notes/index.html
Stefan Sperling wrote on Wed, Sep 29, 2010 at 22:51:43 +0200: > On Wed, Sep 29, 2010 at 08:38:13PM -, stevek...@apache.org wrote: > > Author: steveking > > Date: Wed Sep 29 20:38:13 2010 > > New Revision: 1002839 > > > > URL: http://svn.apache.org/viewvc?rev=1002839&view=rev > > Log: > > * publish/docs/release-notes/index.html > > (Supported versions): new section > > > +1.4.x > > +Sparsely supported > > +only fixes for severe security and data loss issues > > + > > Actually, we didn't even officially fix in 1.4.x the heap overflow bug > which was fixed in 1.6.4 and 1.5.7. I'd consider that a severe issue. > Maybe we should change the wording again to make it match reality? > +1 to changing it such that it matches reality. If this reality happens to be one where 1.4.x doesn't get even security fixes anymore, then +1 to the specific patch, too. :-) > Index: docs/release-notes/1.6.html > === > --- docs/release-notes/1.6.html (revision 1002843) > +++ docs/release-notes/1.6.html (working copy) > @@ -935,8 +935,7 @@ for details. > mean that your 1.4 installation is doomed; if it works well and is all > you need, that's fine. "No longer supported" just means we've stopped > accepting bug reports against 1.4.x versions, and will not make any > -more 1.4.x bugfix releases, except perhaps for absolutely critical > -security or data-loss bugs. > +more 1.4.x bugfix releases. > > > > Index: docs/release-notes/index.html > === > --- docs/release-notes/index.html (revision 1002843) > +++ docs/release-notes/index.html (working copy) > @@ -61,14 +61,10 @@ Subversion releases thus far: > Only fixes for security issues and bugs which could cause data loss > > > -1.4.x > -Sparsely supported > -only fixes for severe security and data loss issues > - > - > -1.3.x and earlier > +1.4.x and earlier > not supported anymore > > + > > >
Re: svn commit: r1002839 - /subversion/site/publish/docs/release-notes/index.html
stevek...@apache.org wrote on Wed, Sep 29, 2010 at 20:38:13 -: > Author: steveking > Date: Wed Sep 29 20:38:13 2010 > New Revision: 1002839 > > URL: http://svn.apache.org/viewvc?rev=1002839&view=rev > Log: > * publish/docs/release-notes/index.html > (Supported versions): new section > > Modified: > subversion/site/publish/docs/release-notes/index.html > > Modified: subversion/site/publish/docs/release-notes/index.html > URL: > http://svn.apache.org/viewvc/subversion/site/publish/docs/release-notes/index.html?rev=1002839&r1=1002838&r2=1002839&view=diff > == > --- subversion/site/publish/docs/release-notes/index.html (original) > +++ subversion/site/publish/docs/release-notes/index.html Wed Sep 29 20:38:13 > 2010 > @@ -16,6 +16,7 @@ > > > > +Release notes > For historical reference, here are a set of release notes for the major > Subversion releases thus far: > > @@ -38,6 +39,39 @@ Subversion releases thus far: > For a detailed history of every release, please see > release history. > > +Supported versions > + > + Might be nice to have some text between these two tags ? e.g., "Project policy is to support the last N release streams. Currently, the following minor streams are supported:" or something similar that uses less confusing terms :-) > + > + > +Version > +Support > +Details > + > + > + > + > +1.6.x > +Fully supported > +fixes for all bugs > + > + > +1.5.x Unmatched closing tag. > +Partially supported > +Only fixes for security issues and bugs which could cause data loss > + > + > +1.4.x > +Sparsely supported > +only fixes for severe security and data loss issues > + > + > +1.3.x and earlier > +not supported anymore > + > + > + > + > > > > >
Re: supported versions
Stefan Küng wrote on Wed, Sep 29, 2010 at 21:42:55 +0200: > Is there a way to get a preview of the modified html files? Showing them > in a browser doesn't show them properly so it's hard to figure out if > the changes are correct. I preview them in a locally-installed httpd. (I already had one set up for the Python tests, so I just extended its config a bit.) I can share that httpd.conf (for previewing the site and running tests) if you like... (really, we should figure out the minimal httpd.conf needed to preview the site and commit that to /site/httpd.conf)
Re: svn commit: r1002839 - /subversion/site/publish/docs/release-notes/index.html
On Wed, Sep 29, 2010 at 08:38:13PM -, stevek...@apache.org wrote: > Author: steveking > Date: Wed Sep 29 20:38:13 2010 > New Revision: 1002839 > > URL: http://svn.apache.org/viewvc?rev=1002839&view=rev > Log: > * publish/docs/release-notes/index.html > (Supported versions): new section > +1.4.x > +Sparsely supported > +only fixes for severe security and data loss issues > + Actually, we didn't even officially fix in 1.4.x the heap overflow bug which was fixed in 1.6.4 and 1.5.7. I'd consider that a severe issue. Maybe we should change the wording again to make it match reality? Index: docs/release-notes/1.6.html === --- docs/release-notes/1.6.html (revision 1002843) +++ docs/release-notes/1.6.html (working copy) @@ -935,8 +935,7 @@ for details. mean that your 1.4 installation is doomed; if it works well and is all you need, that's fine. "No longer supported" just means we've stopped accepting bug reports against 1.4.x versions, and will not make any -more 1.4.x bugfix releases, except perhaps for absolutely critical -security or data-loss bugs. +more 1.4.x bugfix releases. Index: docs/release-notes/index.html === --- docs/release-notes/index.html (revision 1002843) +++ docs/release-notes/index.html (working copy) @@ -61,14 +61,10 @@ Subversion releases thus far: Only fixes for security issues and bugs which could cause data loss -1.4.x -Sparsely supported -only fixes for severe security and data loss issues - - -1.3.x and earlier +1.4.x and earlier not supported anymore +
Re: supported versions
On Wed, Sep 29, 2010 at 09:56:33PM +0200, Stefan Küng wrote: > On 29.09.2010 21:27, Hyrum K. Wright wrote: > > >Yeah, something on the download page is probably appropriate. Wanna > >take a whack at it? > > Patch attached. This looks fine. I'd say just commit it. If there's anything wrong with the way it looks someone (cmpilato?) will happily fix it. Thanks, Stefan
1.6.13 up for signing/testing
1.6.13 tarballs are up for testing and signing. The magic revision is r1002816: http://people.apache.org/~hwright/svn/1.6.13/ As usual, signatures from full committers please send back to me. Testing by enthusiastic users is also welcomed (but remember that this is not yet a blessed release, with all that that implies). If you are a package maintainer, please do not included this release in your distribution until after it has been formally released. I'd like to collect all the signatures in time to do a release by October 1.
Re: [PATCH] for building subversion 1.6.12 for haiku
On Wed, Sep 29, 2010 at 5:45 PM, Philip Martin
wrote:
> Please write a log message:
-
This patch makes use of find_directory() on Haiku to locate the proper
directories to use. Currently B_USER_SETTINGS_DIRECTORY points to
/boot/home/config/settings, so this puts the .subversion directory in
/boot/home/config/settings/subversion instead of $HOME/subversion or
$HOME/.subversion.
Patch by: Scott McCreary (HaikuPorts)
Found by: Chris Roberts (HaikuPorts)
Review by:
---
> http://subversion.apache.org/docs/community-guide/conventions.html#log-messages
>
> Please use spaces rather than tabs for indentation.
>
>> Index: subversion/libsvn_subr/config_file.c
>> ===
>> --- subversion/libsvn_subr/config_file.c (revision 1002716)
>> +++ subversion/libsvn_subr/config_file.c (working copy)
>> @@ -38,6 +38,11 @@
>>
>> #include "svn_private_config.h"
>>
>> +#ifdef __HAIKU__
>> +#include
>> +#include
>> +#endif
>> +
>> /* Used to terminate lines in large multi-line string literals. */
>> #define NL APR_EOL_STR
>>
>> @@ -331,7 +336,19 @@
>> SVN_CONFIG__SUBDIRECTORY, fname, NULL);
>>}
>>
>> -#else /* ! WIN32 */
>> +#elif defined(__HAIKU__)
>> +{
>> + char folder[B_PATH_NAME_LENGTH];
>> +
>> + status_t error = find_directory (B_USER_SETTINGS_DIRECTORY, -1, false,
>> + folder,
>> sizeof(folder));
>> + if (error)
>> + return SVN_NO_ERROR;
>> +
>> + *path_p = svn_path_join_many (pool, folder,
>> +
>> SVN_CONFIG__USR_DIRECTORY, fname, NULL);
>
> Why is that using SVN_CONFIG__USR_DIRECTORY? Should that be __SYS_?
Using __USR_ seems to have the desired effect, perhaps I'm using it
wrong. See other reply below.
>
>> +}
>> +#else /* ! WIN32 && !__HAIKU__ */
>>
>>*path_p = svn_dirent_join_many(pool, SVN_CONFIG__SYS_DIRECTORY, fname,
>> NULL);
>>
>> @@ -1116,8 +1133,21 @@
>> *path = svn_dirent_join_many(pool, folder,
>> SVN_CONFIG__SUBDIRECTORY, fname, NULL);
>>}
>> +
>> +#elif defined(__HAIKU__)
>> +{
>> + char folder[B_PATH_NAME_LENGTH];
>> +
>> + status_t error = find_directory (B_USER_SETTINGS_DIRECTORY, -1, false,
>> + folder,
>> sizeof(folder));
>> + if (error)
>> + return SVN_NO_ERROR;
>> +
>> + *path = svn_path_join_many (pool, folder,
>> +
>> SVN_CONFIG__USR_DIRECTORY, fname, NULL);
>> +}
>> +#else /* ! WIN32 && !__HAIKU__ */
>>
>> -#else /* ! WIN32 */
>>{
>> const char *homedir = svn_user_get_homedir(pool);
>> if (! homedir)
>> Index: subversion/libsvn_subr/config_impl.h
>> ===
>> --- subversion/libsvn_subr/config_impl.h (revision 1002716)
>> +++ subversion/libsvn_subr/config_impl.h (working copy)
>> @@ -114,8 +114,11 @@
>> or svn_config_get_user_config_path() instead. */
>> #ifdef WIN32
>> # define SVN_CONFIG__SUBDIRECTORY"Subversion"
>> -#else /* ! WIN32 */
>> +#elif defined __HAIKU__ /* HAIKU */
>> # define SVN_CONFIG__SYS_DIRECTORY "/etc/subversion"
>
> Should this be used in svn_config__sys_config_path?
>
I'm unclear on what SVN_CONFIG__SYS_DIRECTORY is used for. Through
several attempts I was able to move the $HOME/.subversion directory to
/boot/home/config/settings/subversion which fits in better with how
Haiku stores users' settings. So I left SVN_CONFIG__SYS_DIRECTORY the
same as other OSes for now.
>> +# define SVN_CONFIG__USR_DIRECTORY "subversion"
>> +#else /* ! WIN32 && ! __HAIKU__ */
>> +# define SVN_CONFIG__SYS_DIRECTORY "/etc/subversion"
>> # define SVN_CONFIG__USR_DIRECTORY ".subversion"
>> #endif /* WIN32 */
>>
>
> --
> Philip
>
Attached is an updated version of the patch, fixing the tabs/spaces
issue, and a renamed function that I caught.
-scottmc
Index: libsvn_subr/config_file.c
===
--- libsvn_subr/config_file.c (revision 1002735)
+++ libsvn_subr/config_file.c (working copy)
@@ -38,6 +38,11 @@
#include "svn_private_config.h"
+#ifdef __HAIKU__
+# include
+# include
+#endif
+
/* Used to terminate lines in large multi-line string literals. */
#define NL APR_EOL_STR
@@ -331,8 +336,20 @@
SVN_CONFIG__SUBDIRECTORY, fname, NULL);
}
-#else /* ! WIN32 */
+#elif defined(__HAIKU__)
+ {
+char folder[B_PATH_NAME_LENGTH];
+status_t error = find_directory(B_USER_SETTINGS_DIRECTORY, -1, false,
+folder, sizeof(folder));
+if (error)
+ return SVN_NO_ERROR;
+
+*path_p = svn_dirent_join_many(pool, folder,
+ SVN_CONFIG__USR_DIRECTORY, fname, NULL);
+ }
+#else /* ! WIN32 &&
Re: supported versions
On 29.09.2010 21:27, Hyrum K. Wright wrote: Yeah, something on the download page is probably appropriate. Wanna take a whack at it? Patch attached. Stefan -- ___ oo // \\ "De Chelonian Mobile" (_,\/ \_/ \ TortoiseSVN \ \_/_\_/>The coolest Interface to (Sub)Version Control /_/ \_\ http://tortoisesvn.net [[[ * publish/docs/release-notes/index.html (Supported versions): new section ]]] Index: docs/release-notes/index.html === --- docs/release-notes/index.html (revision 1002821) +++ docs/release-notes/index.html (working copy) @@ -16,6 +16,7 @@ +Release notes For historical reference, here are a set of release notes for the major Subversion releases thus far: @@ -38,6 +39,39 @@ For a detailed history of every release, please see release history. +Supported versions + + + + +Version +Support +Details + + + + +1.6.x +Fully supported +fixes for all bugs + + +1.5.x +Partially supported +Only fixes for security issues and bugs which could cause data loss + + +1.4.x +Sparsely supported +only fixes for severe security and data loss issues + + +1.3.x and earlier +not supported anymore + + + +
Re: supported versions
On 29.09.2010 21:27, Hyrum K. Wright wrote: Not the easiest to find, but: http://subversion.apache.org/docs/release-notes/1.6.html#svn-1.4-deprecation Thanks, this really isn't easy to find. May I suggest to add a separate page explaining in detail which versions are supported and with what (bugfixes, security fixes, data-loss fixes, ...). Or maybe a separate section on the download page? Yeah, something on the download page is probably appropriate. Wanna take a whack at it? Where would be the best place? http://subversion.apache.org/docs/release-notes/ http://subversion.apache.org/source-code.html or http://subversion.apache.org/packages.html IMHO the release-notes page would be best, since the packages page first mentions that those are not officially endorsed or maintained, and the source-code page won't be browsed to by most users. My suggestion for the text: Supported versions The Subversion project supports the following stable branches: (table) 1.6.x fully supported 1.5.x partially supported, only security fixes and fixes which would cause data loss 1.4.x sparse support only: only fixes for severe security issue and data loss issues 1.3.x and earlier not supported anymore Is there a way to get a preview of the modified html files? Showing them in a browser doesn't show them properly so it's hard to figure out if the changes are correct. Stefan -- ___ oo // \\ "De Chelonian Mobile" (_,\/ \_/ \ TortoiseSVN \ \_/_\_/>The coolest Interface to (Sub)Version Control /_/ \_\ http://tortoisesvn.net
Re: supported versions
On Wed, Sep 29, 2010 at 08:36:18PM +0200, Stefan Küng wrote: > Hi, > > Just had a discussion with WANdisco about the versions that are > supported for TSVN. Since their customer is using an 1.4.x server I > wanted to check which version SVN still supports, i.e., provides at > least security updates for. But I couldn't find anything on the web > about that. > > I think that this was discussed on the mailing list before that only > the current stable branch is supported with bugfixes, and the one > branch before only with security updates. Meaning now: 1.6.x still > gets bugfixes, and 1.5.x would get security updates if necessary. > > Is this correct? Yes, it is. > And shouldn't there be something on the web mentioning this? Or > maybe there is but I couldn't find it... http://subversion.apache.org/docs/release-notes/1.6.html#svn-1.4-deprecation Stefan
Re: supported versions
On Wed, Sep 29, 2010 at 2:16 PM, Stefan Küng wrote: > On 29.09.2010 20:45, Hyrum K. Wright wrote: >> >> On Wed, Sep 29, 2010 at 1:36 PM, Stefan Küng >> wrote: >>> >>> Hi, >>> >>> Just had a discussion with WANdisco about the versions that are supported >>> for TSVN. Since their customer is using an 1.4.x server I wanted to check >>> which version SVN still supports, i.e., provides at least security >>> updates >>> for. But I couldn't find anything on the web about that. >>> >>> I think that this was discussed on the mailing list before that only the >>> current stable branch is supported with bugfixes, and the one branch >>> before >>> only with security updates. Meaning now: 1.6.x still gets bugfixes, and >>> 1.5.x would get security updates if necessary. >>> >>> Is this correct? >>> >>> And shouldn't there be something on the web mentioning this? Or maybe >>> there >>> is but I couldn't find it... >> >> Not the easiest to find, but: >> >> http://subversion.apache.org/docs/release-notes/1.6.html#svn-1.4-deprecation > > Thanks, this really isn't easy to find. > May I suggest to add a separate page explaining in detail which versions are > supported and with what (bugfixes, security fixes, data-loss fixes, ...). Or > maybe a separate section on the download page? Yeah, something on the download page is probably appropriate. Wanna take a whack at it? -Hyrum
Re: supported versions
On 29.09.2010 20:45, Hyrum K. Wright wrote: On Wed, Sep 29, 2010 at 1:36 PM, Stefan Küng wrote: Hi, Just had a discussion with WANdisco about the versions that are supported for TSVN. Since their customer is using an 1.4.x server I wanted to check which version SVN still supports, i.e., provides at least security updates for. But I couldn't find anything on the web about that. I think that this was discussed on the mailing list before that only the current stable branch is supported with bugfixes, and the one branch before only with security updates. Meaning now: 1.6.x still gets bugfixes, and 1.5.x would get security updates if necessary. Is this correct? And shouldn't there be something on the web mentioning this? Or maybe there is but I couldn't find it... Not the easiest to find, but: http://subversion.apache.org/docs/release-notes/1.6.html#svn-1.4-deprecation Thanks, this really isn't easy to find. May I suggest to add a separate page explaining in detail which versions are supported and with what (bugfixes, security fixes, data-loss fixes, ...). Or maybe a separate section on the download page? Stefan -- ___ oo // \\ "De Chelonian Mobile" (_,\/ \_/ \ TortoiseSVN \ \_/_\_/>The coolest Interface to (Sub)Version Control /_/ \_\ http://tortoisesvn.net
Re: supported versions
On Wed, Sep 29, 2010 at 1:36 PM, Stefan Küng wrote: > Hi, > > Just had a discussion with WANdisco about the versions that are supported > for TSVN. Since their customer is using an 1.4.x server I wanted to check > which version SVN still supports, i.e., provides at least security updates > for. But I couldn't find anything on the web about that. > > I think that this was discussed on the mailing list before that only the > current stable branch is supported with bugfixes, and the one branch before > only with security updates. Meaning now: 1.6.x still gets bugfixes, and > 1.5.x would get security updates if necessary. > > Is this correct? > > And shouldn't there be something on the web mentioning this? Or maybe there > is but I couldn't find it... Not the easiest to find, but: http://subversion.apache.org/docs/release-notes/1.6.html#svn-1.4-deprecation -Hyrum
supported versions
Hi, Just had a discussion with WANdisco about the versions that are supported for TSVN. Since their customer is using an 1.4.x server I wanted to check which version SVN still supports, i.e., provides at least security updates for. But I couldn't find anything on the web about that. I think that this was discussed on the mailing list before that only the current stable branch is supported with bugfixes, and the one branch before only with security updates. Meaning now: 1.6.x still gets bugfixes, and 1.5.x would get security updates if necessary. Is this correct? And shouldn't there be something on the web mentioning this? Or maybe there is but I couldn't find it... Stefan -- ___ oo // \\ "De Chelonian Mobile" (_,\/ \_/ \ TortoiseSVN \ \_/_\_/>The coolest Interface to (Sub)Version Control /_/ \_\ http://tortoisesvn.net
Re: [PATCH] for building subversion 1.6.12 for haiku
scott mc writes:
Please write a log message:
http://subversion.apache.org/docs/community-guide/conventions.html#log-messages
Please use spaces rather than tabs for indentation.
> Index: subversion/libsvn_subr/config_file.c
> ===
> --- subversion/libsvn_subr/config_file.c (revision 1002716)
> +++ subversion/libsvn_subr/config_file.c (working copy)
> @@ -38,6 +38,11 @@
>
> #include "svn_private_config.h"
>
> +#ifdef __HAIKU__
> +#include
> +#include
> +#endif
> +
> /* Used to terminate lines in large multi-line string literals. */
> #define NL APR_EOL_STR
>
> @@ -331,7 +336,19 @@
> SVN_CONFIG__SUBDIRECTORY, fname, NULL);
>}
>
> -#else /* ! WIN32 */
> +#elif defined(__HAIKU__)
> +{
> + char folder[B_PATH_NAME_LENGTH];
> +
> + status_t error = find_directory (B_USER_SETTINGS_DIRECTORY, -1, false,
> + folder, sizeof(folder));
> + if (error)
> + return SVN_NO_ERROR;
> +
> + *path_p = svn_path_join_many (pool, folder,
> +
> SVN_CONFIG__USR_DIRECTORY, fname, NULL);
Why is that using SVN_CONFIG__USR_DIRECTORY? Should that be __SYS_?
> +}
> +#else /* ! WIN32 && !__HAIKU__ */
>
>*path_p = svn_dirent_join_many(pool, SVN_CONFIG__SYS_DIRECTORY, fname,
> NULL);
>
> @@ -1116,8 +1133,21 @@
> *path = svn_dirent_join_many(pool, folder,
> SVN_CONFIG__SUBDIRECTORY, fname, NULL);
>}
> +
> +#elif defined(__HAIKU__)
> +{
> + char folder[B_PATH_NAME_LENGTH];
> +
> + status_t error = find_directory (B_USER_SETTINGS_DIRECTORY, -1, false,
> + folder, sizeof(folder));
> + if (error)
> + return SVN_NO_ERROR;
> +
> + *path = svn_path_join_many (pool, folder,
> +
> SVN_CONFIG__USR_DIRECTORY, fname, NULL);
> +}
> +#else /* ! WIN32 && !__HAIKU__ */
>
> -#else /* ! WIN32 */
>{
> const char *homedir = svn_user_get_homedir(pool);
> if (! homedir)
> Index: subversion/libsvn_subr/config_impl.h
> ===
> --- subversion/libsvn_subr/config_impl.h (revision 1002716)
> +++ subversion/libsvn_subr/config_impl.h (working copy)
> @@ -114,8 +114,11 @@
> or svn_config_get_user_config_path() instead. */
> #ifdef WIN32
> # define SVN_CONFIG__SUBDIRECTORY"Subversion"
> -#else /* ! WIN32 */
> +#elif defined __HAIKU__ /* HAIKU */
> # define SVN_CONFIG__SYS_DIRECTORY "/etc/subversion"
Should this be used in svn_config__sys_config_path?
> +# define SVN_CONFIG__USR_DIRECTORY "subversion"
> +#else /* ! WIN32 && ! __HAIKU__ */
> +# define SVN_CONFIG__SYS_DIRECTORY "/etc/subversion"
> # define SVN_CONFIG__USR_DIRECTORY ".subversion"
> #endif /* WIN32 */
>
--
Philip
Re: [PATCH] for building subversion 1.6.12 for haiku
> Unfortunately this mailing list appears to be silently dropping
> attachments in some cases. Can you try again with a .txt extension?
>
> Also, would it be possible for you to send a patch that's relative
> to Subversion's trunk code, rather than 1.6.12?
>
> Thanks,
> Stefan
>
Sure. Here it is vs. subversion-svn-r-1002716. (also attaching as a
.patch and a .txt in case either of those work better)
-scottmc
Scott McCreary
HaikuPorts
Index: subversion/libsvn_subr/config_file.c
===
--- subversion/libsvn_subr/config_file.c(revision 1002716)
+++ subversion/libsvn_subr/config_file.c(working copy)
@@ -38,6 +38,11 @@
#include "svn_private_config.h"
+#ifdef __HAIKU__
+# include
+# include
+#endif
+
/* Used to terminate lines in large multi-line string literals. */
#define NL APR_EOL_STR
@@ -331,7 +336,19 @@
SVN_CONFIG__SUBDIRECTORY, fname, NULL);
}
-#else /* ! WIN32 */
+#elif defined(__HAIKU__)
+{
+ char folder[B_PATH_NAME_LENGTH];
+
+ status_t error = find_directory (B_USER_SETTINGS_DIRECTORY, -1, false,
+ folder, sizeof(folder));
+ if (error)
+ return SVN_NO_ERROR;
+
+ *path_p = svn_path_join_many (pool, folder,
+
SVN_CONFIG__USR_DIRECTORY, fname, NULL);
+}
+#else /* ! WIN32 && !__HAIKU__ */
*path_p = svn_dirent_join_many(pool, SVN_CONFIG__SYS_DIRECTORY, fname, NULL);
@@ -1116,8 +1133,21 @@
*path = svn_dirent_join_many(pool, folder,
SVN_CONFIG__SUBDIRECTORY, fname, NULL);
}
+
+#elif defined(__HAIKU__)
+{
+ char folder[B_PATH_NAME_LENGTH];
+
+ status_t error = find_directory (B_USER_SETTINGS_DIRECTORY, -1, false,
+ folder, sizeof(folder));
+ if (error)
+ return SVN_NO_ERROR;
+
+ *path = svn_path_join_many (pool, folder,
+
SVN_CONFIG__USR_DIRECTORY, fname, NULL);
+}
+#else /* ! WIN32 && !__HAIKU__ */
-#else /* ! WIN32 */
{
const char *homedir = svn_user_get_homedir(pool);
if (! homedir)
Index: subversion/libsvn_subr/config_impl.h
===
--- subversion/libsvn_subr/config_impl.h(revision 1002716)
+++ subversion/libsvn_subr/config_impl.h(working copy)
@@ -114,8 +114,11 @@
or svn_config_get_user_config_path() instead. */
#ifdef WIN32
# define SVN_CONFIG__SUBDIRECTORY"Subversion"
-#else /* ! WIN32 */
+#elif defined __HAIKU__ /* HAIKU */
# define SVN_CONFIG__SYS_DIRECTORY "/etc/subversion"
+# define SVN_CONFIG__USR_DIRECTORY "subversion"
+#else /* ! WIN32 && ! __HAIKU__ */
+# define SVN_CONFIG__SYS_DIRECTORY "/etc/subversion"
# define SVN_CONFIG__USR_DIRECTORY ".subversion"
#endif /* WIN32 */
Index: subversion/libsvn_subr/config_file.c
===
--- subversion/libsvn_subr/config_file.c(revision 1002716)
+++ subversion/libsvn_subr/config_file.c(working copy)
@@ -38,6 +38,11 @@
#include "svn_private_config.h"
+#ifdef __HAIKU__
+# include
+# include
+#endif
+
/* Used to terminate lines in large multi-line string literals. */
#define NL APR_EOL_STR
@@ -331,7 +336,19 @@
SVN_CONFIG__SUBDIRECTORY, fname, NULL);
}
-#else /* ! WIN32 */
+#elif defined(__HAIKU__)
+{
+ char folder[B_PATH_NAME_LENGTH];
+
+ status_t error = find_directory (B_USER_SETTINGS_DIRECTORY, -1, false,
+ folder, sizeof(folder));
+ if (error)
+ return SVN_NO_ERROR;
+
+ *path_p = svn_path_join_many (pool, folder,
+
SVN_CONFIG__USR_DIRECTORY, fname, NULL);
+}
+#else /* ! WIN32 && !__HAIKU__ */
*path_p = svn_dirent_join_many(pool, SVN_CONFIG__SYS_DIRECTORY, fname, NULL);
@@ -1116,8 +1133,21 @@
*path = svn_dirent_join_many(pool, folder,
SVN_CONFIG__SUBDIRECTORY, fname, NULL);
}
+
+#elif defined(__HAIKU__)
+{
+ char folder[B_PATH_NAME_LENGTH];
+
+ status_t error = find_directory (B_USER_SETTINGS_DIRECTORY, -1, false,
+ folder, sizeof(folder));
+ if (error)
+ return SVN_NO_ERROR;
+
+ *path = svn_path_join_many (pool, folder,
+
SVN_CONFIG__USR_DIRECTORY, fname, NULL);
+}
+#else /* ! WIN32 && !__HAIKU__ */
-#else /* ! WIN32 */
{
const char *homedir = svn_user_get_homedir(pool);
if (! homedir)
Index: subversion/libsvn_subr/config_impl.h
===
--- subversion/libsvn_subr/config_impl.h(revision 1002716)
+++ subversion
Re: [PATCH] for building subversion 1.6.12 for haiku
On Wed, Sep 29, 2010 at 07:37:27AM -0700, scott mc wrote: > Here is the patch we use for building subversion on Haiku. > -scottmc > > Scott McCreary > HaikuPorts Unfortunately this mailing list appears to be silently dropping attachments in some cases. Can you try again with a .txt extension? Also, would it be possible for you to send a patch that's relative to Subversion's trunk code, rather than 1.6.12? Thanks, Stefan
[PATCH] for building subversion 1.6.12 for haiku
Here is the patch we use for building subversion on Haiku. -scottmc Scott McCreary HaikuPorts
What happened to svndiff1
Hi, I'd like to know what happened to svndiff version 1 format. All the specs and code including notes/dump-load-format.txt seem to refer only to svndiff0. -- Ram
Re: [PATCH] don't do autoprops on symbolic links
On Wed, Sep 29, 2010 at 10:56:17AM +1000, Gavin Beau Baumanis wrote: > As a result of no comments for this patch , I have logged it into the issue > tracker so that it doesn't get lost. > > http://subversion.tigris.org/issue-tracker.html > Issue Number : #3722 I've committed the patch. Thanks very much! Stefan
Re: svn commit: r1002151 - in /subversion/trunk/subversion: libsvn_auth_kwallet/kwallet.cpp svn/main.c svnsync/main.c
On Tue, Sep 28, 2010 at 11:21:26AM -0700, Blair Zajac wrote:
> On 9/28/2010 6:07 AM, s...@apache.org wrote:
> >Author: stsp
> >Date: Tue Sep 28 13:07:23 2010
> >New Revision: 1002151
> >
> >URL: http://svn.apache.org/viewvc?rev=1002151&view=rev
> >Log:
> >Remove some goo introduced in r878078 and follow-ups, which was related to
> >the Linux-specific code which has been removed in r1002144.
>
> >- INITIALIZE_APPLICATION
> >+ QCoreApplication *app;
> >+ if (! qApp)
> >+{
> >+ int argc = 1;
> >+ app = new QCoreApplication(argc, (char *[1]) {(char *) "svn"});
> >+}
>
> Out of curiosity, what does this do?
No idea. I didn't understand this either.
I moved this code back out of the INITIALIZE_APPLICATION macro anyway.
The code was this way before the INITIALIZE_APPLICATION macro was
introduced.
> QCoreApplication *app isn't static, so does this do some setup logic
> that we don't need to keep track of?
That's what it looks like to me, too.
I suppose Arfrever will definitely know what this is doing.
Stefan
Re: [PATCH] Use neon's system proxy detection if not explicitly specified
On Wed, 2010-09-29 at 12:42 +0200, Daniel Shahaf wrote: > > > > 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. > > The latter please; when Neon 0.39 comes around we'll fix all checks at > the same time. In this case I would consider my patch complete, as this is already what has been submitted. Thanks for your confirmation; Is there any further action to be taken by myself for this to happen? Dominique
Re: [PATCH] Use neon's system proxy detection if not explicitly specified
Dominique Leuenberger wrote on Mon, Sep 27, 2010 at 14:48:20 +0200: > 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. The latter please; when Neon 0.39 comes around we'll fix all checks at the same time.
RE: [PATCH] Use neon's system proxy detection if not explicitly specified
Dominique wrote: > 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. > > As my last mail did remain unanswered and I don't want to have the > whole thread go lost; here re-replied: > > The current check for 0.29/0.3x is copied from the other checks for > NEON_0_2(<9). I think it would not be wrong to keep the checks in sync > (or optionally even restructure this test with nested ifs). > > 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. This is something that the committers need to decide on. As I'm not a committer, I won't express an opinion. 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 __
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. As my last mail did remain unanswered and I don't want to have the whole thread go lost; here re-replied: The current check for 0.29/0.3x is copied from the other checks for NEON_0_2(<9). I think it would not be wrong to keep the checks in sync (or optionally even restructure this test with nested ifs). 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. Best regards, Dominique
Re: add NODES.prior_deleted boolean column
Greg Stein writes: > To do this, it seems that we're going to need to expose (from wc_db.h) > a structure containing "all" the row data. Thankfully, this big ol' > structure is *private* to libsvn_wc, and we can change it at will > (unlike svn_wc_status_t). I would suggest that we use a callback -- > wc_db can step through each row of the result, fill in the structure, > and execute a callback (clearing an iterpool on each "step" with the > cursor). Yes, the caching is private to libsvn_wc. It might even be private to status within libsvn_wc initially. > The one tricky part to a callback is eliding all but the highest > op_depth. Does an ORDER BY in the query affect its runtime at all? I had the following indices on NODES: PRIMARY KEY(wc_id, local_relpath, op_depth) CREATE INDEX i_parent ON NODES (wc_id, parent_relpath, local_relpath) if I change the i_parent index to CREATE INDEX i_parent ON NODES (wc_id, parent_relpath, local_relpath, op_depth) then the order by on this query SELECT ... FROM NODES WHERE wc_id = ?1 AND parent_relpath = ?2 ORDER BY local_relpath, op_depth doesn't add a significant overhead. -- Philip
Re: Some tips on profiling
Hi Philip, Philip Martin writes: > The performance of svnrdump is likely to be dominated by IO from the > repository, network or disk depending on the RA layer. strace is a > useful tool to see opens/reads/writes. You can see what order the > calls occur, how many there are, how big they are and how long they > take. Ah, thanks for the tip. > Valgrind/Callgrind is good and doesn't require you to instrument the > code, but it does help to build with debug information. It does > impose a massive runtime overhead. I don't mind -- I'm mostly using some remote machines to gather the profiling data :) > This is what I get when dumping 1000 revisions from a local mirror of > the Subversion repository over ra_neon: > > CPU: Core 2, speed 1200 MHz (estimated) > Counted CPU_CLK_UNHALTED events (Clock cycles when not halted) with a unit > mask of 0x00 (Unhalted core cycles) count 10 > samples %app name symbol name > 4738 41.1893 no-vmlinux (no symbols) > 1037 9.0150 libxml2.so.2.6.32(no symbols) > 700 6.0854 libneon.so.27.1.2(no symbols) > 238 2.0690 libc-2.7.so _int_malloc > 228 1.9821 libc-2.7.so memcpy > 221 1.9212 libc-2.7.so memset > 217 1.8865 libc-2.7.so strlen > 191 1.6604 libsvn_subr-1.so.0.0.0 decode_bytes > 180 1.5648 libc-2.7.so vfprintf > 171 1.4866 libc-2.7.so strcmp > 153 1.3301 libapr-1.so.0.2.12 apr_hashfunc_default > 134 1.1649 libapr-1.so.0.2.12 apr_vformatter > 130 1.1301 libapr-1.so.0.2.12 apr_palloc > > That's on my Debian desktop. At the recent Apache Retreat I tried to > demonstrate OProfile on my Ubuntu laptop and could not get it to work > properly, probably because I forgot about -fno-omit-frame-pointer. Ah, now I see why it didn't work for me. The data from Callgrind is very interesting- it seems to suggest that APR hashtables are prohibitively expensive. @Stefan: Thoughts on hacking APR hashtables directly? > Finally there is traditional gprof. It's a long time since I used it > so I don't remember the details. You instrument the code at compile > time using CFLAGS=-pg. If an instrumented function foo calls into a > library bar that is not instrumented then bar is invisible, all you > see is how long foo took to execute. Yes, I used gprof initially. Callgrind is WAY more useful. -- Ram
Re: svn commit: r1002503 - /subversion/trunk/subversion/svnrdump/dump_editor.c
Hi Daniel, Daniel Shahaf writes: > > == > > --- subversion/trunk/subversion/svnrdump/dump_editor.c (original) > > +++ subversion/trunk/subversion/svnrdump/dump_editor.c Wed Sep 29 07:52:55 > > 2010 > > @@ -143,8 +143,7 @@ make_dir_baton(const char *path, > >new_db->eb = eb; > >new_db->parent_dir_baton = pb; > >new_db->abspath = abspath; > > - new_db->copyfrom_path = copyfrom_path ? > > -apr_pstrdup(pool, copyfrom_path) : NULL; > > + new_db->copyfrom_path = copyfrom_path; > > Does this function now assume that COPYFROM_PATH has a certain lifetime? > If so, should that assumption go in the docstring? The function simply sets a parameter: there are many functions that assume many things, so I think it would be best to document it in the struct itself. Thanks for this review :) -- Ram
Re: svn commit: r1002470 - /subversion/trunk/subversion/svnrdump/dump_editor.c
Hi Daniel, Daniel Shahaf writes: > So, you use a top-level pool but never destroy it? > > That's not good. > > Can you please find another solution? (Either get the caller to > guarantee something about the lifetime of the pool they provide (there > is precedent for this), or figure out why close_edit() isn't called (and > possibly patch the faulty driver). Fixed in r1002502 :) Thanks for the review. -- Ram
Re: svn commit: r1002503 - /subversion/trunk/subversion/svnrdump/dump_editor.c
artag...@apache.org wrote on Wed, Sep 29, 2010 at 07:52:55 -: > Author: artagnon > Date: Wed Sep 29 07:52:55 2010 > New Revision: 1002503 > > URL: http://svn.apache.org/viewvc?rev=1002503&view=rev > Log: > svnrdump: dump_editor: Avoid duplicating strings unnecessarily > > * subversion/svnrdump/dump_editor.c > > (open_directory, close_directory, make_dir_baton): Instead of first > allocating `copyfrom_path` in `pool` and then copying it to > `eb->pool`, allocate it in `eb->pool` in the first place. > > Modified: > subversion/trunk/subversion/svnrdump/dump_editor.c > > Modified: subversion/trunk/subversion/svnrdump/dump_editor.c > URL: > http://svn.apache.org/viewvc/subversion/trunk/subversion/svnrdump/dump_editor.c?rev=1002503&r1=1002502&r2=1002503&view=diff > == > --- subversion/trunk/subversion/svnrdump/dump_editor.c (original) > +++ subversion/trunk/subversion/svnrdump/dump_editor.c Wed Sep 29 07:52:55 > 2010 > @@ -143,8 +143,7 @@ make_dir_baton(const char *path, >new_db->eb = eb; >new_db->parent_dir_baton = pb; >new_db->abspath = abspath; > - new_db->copyfrom_path = copyfrom_path ? > -apr_pstrdup(pool, copyfrom_path) : NULL; > + new_db->copyfrom_path = copyfrom_path; Does this function now assume that COPYFROM_PATH has a certain lifetime? If so, should that assumption go in the docstring? >new_db->copyfrom_rev = copyfrom_rev; >new_db->added = added; >new_db->written_out = FALSE;
Re: svn commit: r1002502 - /subversion/trunk/subversion/svnrdump/dump_editor.c
artag...@apache.org wrote on Wed, Sep 29, 2010 at 07:44:48 -:
> Author: artagnon
> Date: Wed Sep 29 07:44:48 2010
> New Revision: 1002502
>
> URL: http://svn.apache.org/viewvc?rev=1002502&view=rev
> Log:
> svnrdump: dump_editor: Followup r1002470 to avoid creating a toplevel
> pool by changing the function that creates the per-revision pool.
>
> * subversion/svnrdump/dump_editor.c
>
> (open_root): Don't create the per-revision pool here. Simply clear
> it after each iteration here.
>
> (get_dump_editor): Create the per-revision pool `eb->pool` here and
> make it a subpool of `pool` so that it's cleaned up when `pool` is
> cleaned up.
>
> Modified:
> subversion/trunk/subversion/svnrdump/dump_editor.c
>
> Modified: subversion/trunk/subversion/svnrdump/dump_editor.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/svnrdump/dump_editor.c?rev=1002502&r1=1002501&r2=1002502&view=diff
> ==
> --- subversion/trunk/subversion/svnrdump/dump_editor.c (original)
> +++ subversion/trunk/subversion/svnrdump/dump_editor.c Wed Sep 29 07:44:48
> 2010
> @@ -368,11 +368,8 @@ open_root(void *edit_baton,
> {
>struct dump_edit_baton *eb = edit_baton;
>
> - /* Special toplevel per-revision pool */
> - if (eb->pool)
> -svn_pool_clear(eb->pool);
> - else
> -eb->pool = svn_pool_create(NULL);
> + /* Clear the per-revision pool after each revision */
> + svn_pool_clear(eb->pool);
>
Need to check for NULL? apr_pool_clear() docs don't bless passing NULL.
>eb->props = apr_hash_make(eb->pool);
>eb->deleted_props = apr_hash_make(eb->pool);
> @@ -852,6 +849,9 @@ get_dump_editor(const svn_delta_editor_t
>eb = apr_pcalloc(pool, sizeof(struct dump_edit_baton));
>eb->stream = stream;
>
> + /* Create a special per-revision pool */
> + eb->pool = svn_pool_create(pool);
> +
So, now the per-revision pool is a subpool of get_dump_editor()'s pool.
Okay.
>de = svn_delta_default_editor(pool);
>de->open_root = open_root;
>de->delete_entry = delete_entry;
>
>
By the way, not directly related to this commit, but do note that we've
last year started with a dual-pool paradigm (where functions take both
a result_pool and a scratch_pool); it's documented in HACKING and in
plenty of examples in new code. Please use that paradigm where
appropriate. Thanks.
Daniel
Re: svn commit: r1002470 - /subversion/trunk/subversion/svnrdump/dump_editor.c
So, you use a top-level pool but never destroy it?
That's not good.
Can you please find another solution? (Either get the caller to
guarantee something about the lifetime of the pool they provide (there
is precedent for this), or figure out why close_edit() isn't called (and
possibly patch the faulty driver).
artag...@apache.org wrote on Wed, Sep 29, 2010 at 05:08:42 -:
> Author: artagnon
> Date: Wed Sep 29 05:08:42 2010
> New Revision: 1002470
>
> URL: http://svn.apache.org/viewvc?rev=1002470&view=rev
> Log:
> svnrdump: dump_editor: Fix issues related to per-revision pool
>
> * subversion/svnrdump/dump_editor.c
>
> (open_root): Mention that `eb->pool` is a per-revision pool and
> clear it in every iteration. Make it a toplevel pool since the
> lifetime of `pool` is not known.
>
> (close_edit): Remove useless svn_pool_destroy cruft since logs show
> that this function is never actually called.
>
> Modified:
> subversion/trunk/subversion/svnrdump/dump_editor.c
>
> Modified: subversion/trunk/subversion/svnrdump/dump_editor.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/svnrdump/dump_editor.c?rev=1002470&r1=1002469&r2=1002470&view=diff
> ==
> --- subversion/trunk/subversion/svnrdump/dump_editor.c (original)
> +++ subversion/trunk/subversion/svnrdump/dump_editor.c Wed Sep 29 05:08:42
> 2010
> @@ -367,10 +367,13 @@ open_root(void *edit_baton,
>void **root_baton)
> {
>struct dump_edit_baton *eb = edit_baton;
> - /* Allocate a special pool for the edit_baton to avoid pool
> - lifetime issues */
>
> - eb->pool = svn_pool_create(pool);
> + /* Special toplevel per-revision pool */
> + if (eb->pool)
> +svn_pool_clear(eb->pool);
> + else
> +eb->pool = svn_pool_create(NULL);
> +
>eb->props = apr_hash_make(eb->pool);
>eb->deleted_props = apr_hash_make(eb->pool);
>eb->propstring = svn_stringbuf_create("", eb->pool);
> @@ -833,10 +836,6 @@ close_file(void *file_baton,
> static svn_error_t *
> close_edit(void *edit_baton, apr_pool_t *pool)
> {
> - struct dump_edit_baton *eb = edit_baton;
> - LDR_DBG(("close_edit\n"));
> - svn_pool_destroy(eb->pool);
> -
>return SVN_NO_ERROR;
> }
>
>
>
