# [PATCH] Reintroduce svn pools to solve the memory leak. 4 messages from 2006-03-27 to 2006-03-28. Participants: Santi Béjar, Junio C Hamano, Jan-Benedict Glaw, Karl Hasselström. Thread: https://gitlist.dev/t/3736 ## Santi Béjar, 2006-03-27 11:26 Subject: [PATCH] Reintroduce svn pools to solve the memory leak. Message-ID: <8aa486160603270326i3a8ddcfau61ca84cdac036ff9@mail.gmail.com> URL: https://gitlist.dev/e/8aa486160603270326i3a8ddcfau61ca84cdac036ff9%40mail.gmail.com ``` On 3/24/06, Santi Béjar wrote: > Jan-Benedict Glaw writes: > > > On Wed, 2006-03-22 14:33:37 +0100, Jan-Benedict Glaw wrote: > > > > Since it seems nobody looked at the GCC import run (which means to use > > the svnimport), I ran it again, under strace control: > > > >> GCC > >> ~~~ > >> $ /home/jbglaw/bin/git svnimport -C gcc -v svn://gcc.gnu.org/svn/gcc > > > >> Committed change 3936:/ 1993-03-31 05:44:03) > >> Commit ID ceff85145f8671fb2a9d826a761cedc2a507bd1e > >> Writing to refs/heads/origin > >> DONE: 3936 origin ceff85145f8671fb2a9d826a761cedc2a507bd1e > >> ... 3937 trunk/gcc/final.c ... > >> Can't fork at /home/jbglaw/bin/git-svnimport line 379. > > > > I have the same (?) problem with one of my svn repository. It worked > before (I've redone the import with the -r flag), so I bisected it. > The problematic commit seems to be: > > diff-tree 4802426... (from 525c0d7...) > Author: Karl Hasselström > Date: Sun Feb 26 06:11:27 2006 +0100 > > svnimport: Convert executable flag > > Convert the svn:executable property to file mode 755 when converting > an SVN repository to GIT. > > Signed-off-by: Karl Hasselström > Signed-off-by: Junio C Hamano > > :100755 100755 ee2940f... 6603b96... M git-svnimport.perl > > I think it has a memory leak, it used up to 140m of memory. > > $ git reset --hard 4802426^ > $ time ../git-svnimport.perl file:///path/ > Use of uninitialized value in string eq at ../git-svnimport.perl line 463. > Use of uninitialized value in substitution (s///) at ../git-svnimport.perl line 466. > real 0m55.801s > user 0m30.578s > sys 0m23.084s > > $ git reset --hard 4802426 > $ time ../git-svnimport.perl file:///path/ > Use of uninitialized value in string eq at ../git-svnimport.perl line 463. > Use of uninitialized value in substitution (s///) at ../git-svnimport.perl line 466. > Can't fork at /home/santi/usr/src/scm/git/git-svnimport.perl line 331. > real 6m2.163s > user 0m20.332s > sys 0m50.180s > > and it didn't finished. Hope it helps. And this patch fixes my problems. --- Introduced in 4802426. Signed-off-by: Santi Béjar --- git-svnimport.perl | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/git-svnimport.perl b/git-svnimport.perl index 639aa41..f2cf062 100755 --- a/git-svnimport.perl +++ b/git-svnimport.perl @@ -135,8 +135,10 @@ print "... $rev $path ...\n" if $opt_v; my (undef, $properties); + my $pool = SVN::Pool->new(); eval { (undef, $properties) - = $self->{'svn'}->get_file($path,$rev,$fh); }; + = $self->{'svn'}->get_file($path,$rev,$fh,$pool); }; + $pool->clear; if($@) { return undef if $@ =~ /Attempted to get checksum/; die $@; ``` ## Junio C Hamano, 2006-03-27 18:16 Subject: Re: [PATCH] Reintroduce svn pools to solve the memory leak. Message-ID: <7vhd5joiqt.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vhd5joiqt.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <8aa486160603270326i3a8ddcfau61ca84cdac036ff9@mail.gmail.com> ``` "Santi Béjar" writes: > On 3/24/06, Santi Béjar wrote: >> Jan-Benedict Glaw writes: >> >> diff-tree 4802426... (from 525c0d7...) >> Author: Karl Hasselström >> Date: Sun Feb 26 06:11:27 2006 +0100 >> >> svnimport: Convert executable flag >> >> Convert the svn:executable property to file mode 755 when converting >> an SVN repository to GIT. >> >> Signed-off-by: Karl Hasselström >> Signed-off-by: Junio C Hamano >> >> :100755 100755 ee2940f... 6603b96... M git-svnimport.perl > > And this patch fixes my problems. Jan-Benedict, thanks for pinpointing the regression, and Santi, thanks for the patch. I should have looked a bit more closely when applying the patch -- it is clear that the patch is doing more than what its log says. My fault. Karl, were there other reasons you needed to disable the pool here (maybe to work around a problem with incompatible version of SVN module)? I see some other uses of SVN::Pool still there in the code, so I am assuming this was a simple typo, but just in case... ``` ## Jan-Benedict Glaw, 2006-03-28 08:10 Subject: Re: [PATCH] Reintroduce svn pools to solve the memory leak. Message-ID: <20060328081037.GH31387@lug-owl.de> URL: https://gitlist.dev/e/20060328081037.GH31387%40lug-owl.de In-Reply-To: <7vhd5joiqt.fsf@assigned-by-dhcp.cox.net> ``` On Mon, 2006-03-27 10:16:58 -0800, Junio C Hamano wrote: > "Santi Béjar" writes: > > On 3/24/06, Santi Béjar wrote: > >> Jan-Benedict Glaw writes: > >> > >> diff-tree 4802426... (from 525c0d7...) > >> Author: Karl Hasselström > >> Date: Sun Feb 26 06:11:27 2006 +0100 > >> > >> svnimport: Convert executable flag > >> > >> Convert the svn:executable property to file mode 755 when converting > >> an SVN repository to GIT. > >> > >> Signed-off-by: Karl Hasselström > >> Signed-off-by: Junio C Hamano > >> > >> :100755 100755 ee2940f... 6603b96... M git-svnimport.perl > > > > And this patch fixes my problems. > > Jan-Benedict, thanks for pinpointing the regression, and Santi, > thanks for the patch. I'm currently running another test with GCC: this patch cuts down memory consumption to less than 1/10 of the previous state (VSZ) and even more for RSS (my system is quite loaded...) MfG, JBG -- Jan-Benedict Glaw jbglaw@lug-owl.de . +49-172-7608481 _ O _ "Eine Freie Meinung in einem Freien Kopf | Gegen Zensur | Gegen Krieg _ _ O für einen Freien Staat voll Freier Bürger" | im Internet! | im Irak! O O O ret = do_actions((curr | FREE_SPEECH) & ~(NEW_COPYRIGHT_LAW | DRM | TCPA)); ``` ## Karl Hasselström, 2006-03-28 12:20 Subject: Re: [PATCH] Reintroduce svn pools to solve the memory leak. Message-ID: <20060328122031.GA14124@diana.vm.bytemark.co.uk> URL: https://gitlist.dev/e/20060328122031.GA14124%40diana.vm.bytemark.co.uk In-Reply-To: <7vhd5joiqt.fsf@assigned-by-dhcp.cox.net> ``` On 2006-03-27 10:16:58 -0800, Junio C Hamano wrote: > Karl, were there other reasons you needed to disable the pool here > (maybe to work around a problem with incompatible version of SVN > module)? I see some other uses of SVN::Pool still there in the code, > so I am assuming this was a simple typo, but just in case... No, it's just a simple mistake (the mistake being me not realizing why an explicit pool was needed, and simply dropping it when things worked fine without it). -- Karl Hasselström, kha@treskal.com www.treskal.com/kalle ```