{"thread":{"id":"9994","subject":"[PATCH] resend: really plug memory leaks in git-svnimport","startedAt":"2007-09-24T10:57:40Z","lastAt":"2007-09-25T18:37:52Z","messageCount":3,"participants":["Stefan Sperling","Junio C Hamano","Andreas Ericsson"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"53907","messageId":"20070924105740.GB8900@ted","threadId":"9994","inReplyTo":null,"subject":"[PATCH] resend: really plug memory leaks in git-svnimport","fromName":"Stefan Sperling","fromEmail":"stsp@elego.de","sentAt":"2007-09-24T10:57:40Z","receivedAt":"2007-09-24T10:57:40Z","isPatch":true,"sender":{"key":"stsp@elego.de","avatar":"https://avatars.githubusercontent.com/u/9281333?v=4"},"body":"Junio asked me to resend this patch to the mailing list.\n\nThis version of the patch is adjusted to apply cleanly\nto current HEAD.\n\n@Junio: I'm not resending the multiple branch/tag dirs patch\njust yet, because I want to polish it first -- I've got another\nidea how to improve it.\n\nLog message:\n\nFix pool handling in git-svnimport to avoid memory leaks.\n\n- Create an explicit one-and-only root pool.\n- Closely follow examples in SVN::Core man page.\n  Before calling a subversion function, create a subpool of our\n  root pool and make it the new default pool. \n- Create a subpool for looping over svn revisions and clear\n  this subpool (i.e. it mark for reuse, don't decallocate it)\n  at the start of the loop instead of allocating new memory\n  with each iteration.\n\nSee http://marc.info/?l=git&m=118554191513822&w=2 for a detailed\nexplanation of the issue.\n\nSigned-off-by: Stefan Sperling <stsp@elego.de>\n\ndiff --git a/git-svnimport.perl b/git-svnimport.perl\nindex aa5b3b2..ea8c1b2 100755\n--- a/git-svnimport.perl\n+++ b/git-svnimport.perl\n@@ -54,6 +54,7 @@ my $branch_name = $opt_b || \"branches\";\n my $project_name = $opt_P || \"\";\n $project_name = \"/\" . $project_name if ($project_name);\n my $repack_after = $opt_R || 1000;\n+my $root_pool = SVN::Pool->new_default;\n \n @ARGV == 1 or @ARGV == 2 or usage();\n \n@@ -132,7 +133,7 @@ sub conn {\n \tmy $auth = SVN::Core::auth_open ([SVN::Client::get_simple_provider,\n \t\t\t  SVN::Client::get_ssl_server_trust_file_provider,\n \t\t\t  SVN::Client::get_username_provider]);\n-\tmy $s = SVN::Ra->new(url => $repo, auth => $auth);\n+\tmy $s = SVN::Ra->new(url => $repo, auth => $auth, pool => $root_pool);\n \tdie \"SVN connection to $repo: $!\\n\" unless defined $s;\n \t$self->{'svn'} = $s;\n \t$self->{'repo'} = $repo;\n@@ -147,11 +148,10 @@ sub file {\n \n \tprint \"... $rev $path ...\\n\" if $opt_v;\n \tmy (undef, $properties);\n-\tmy $pool = SVN::Pool->new();\n \t$path =~ s#^/*##;\n+\tmy $subpool = SVN::Pool::new_default_sub;\n \teval { (undef, $properties)\n-\t\t   = $self->{'svn'}->get_file($path,$rev,$fh,$pool); };\n-\t$pool->clear;\n+\t\t   = $self->{'svn'}->get_file($path,$rev,$fh); };\n \tif($@) {\n \t\treturn undef if $@ =~ /Attempted to get checksum/;\n \t\tdie $@;\n@@ -185,6 +185,7 @@ sub ignore {\n \n \tprint \"... $rev $path ...\\n\" if $opt_v;\n \t$path =~ s#^/*##;\n+\tmy $subpool = SVN::Pool::new_default_sub;\n \tmy (undef,undef,$properties)\n \t    = $self->{'svn'}->get_dir($path,$rev,undef);\n \tif (exists $properties->{'svn:ignore'}) {\n@@ -202,6 +203,7 @@ sub ignore {\n sub dir_list {\n \tmy($self,$path,$rev) = @_;\n \t$path =~ s#^/*##;\n+\tmy $subpool = SVN::Pool::new_default_sub;\n \tmy ($dirents,undef,$properties)\n \t    = $self->{'svn'}->get_dir($path,$rev,undef);\n \treturn $dirents;\n@@ -358,10 +360,9 @@ open BRANCHES,\">>\", \"$git_dir/svn2git\";\n \n sub node_kind($$) {\n \tmy ($svnpath, $revision) = @_;\n-\tmy $pool=SVN::Pool->new;\n \t$svnpath =~ s#^/*##;\n-\tmy $kind = $svn->{'svn'}->check_path($svnpath,$revision,$pool);\n-\t$pool->clear;\n+\tmy $subpool = SVN::Pool::new_default_sub;\n+\tmy $kind = $svn->{'svn'}->check_path($svnpath,$revision);\n \treturn $kind;\n }\n \n@@ -889,7 +890,7 @@ sub commit_all {\n \t# Recursive use of the SVN connection does not work\n \tlocal $svn = $svn2;\n \n-\tmy ($changed_paths, $revision, $author, $date, $message, $pool) = @_;\n+\tmy ($changed_paths, $revision, $author, $date, $message) = @_;\n \tmy %p;\n \twhile(my($path,$action) = each %$changed_paths) {\n \t\t$p{$path} = [ $action->action,$action->copyfrom_path, $action->copyfrom_rev, $path ];\n@@ -925,14 +926,14 @@ print \"Processing from $current_rev to $opt_l ...\\n\" if $opt_v;\n my $from_rev;\n my $to_rev = $current_rev - 1;\n \n+my $subpool = SVN::Pool::new_default_sub;\n while ($to_rev < $opt_l) {\n+\t$subpool->clear;\n \t$from_rev = $to_rev + 1;\n \t$to_rev = $from_rev + $repack_after;\n \t$to_rev = $opt_l if $opt_l < $to_rev;\n \tprint \"Fetching from $from_rev to $to_rev ...\\n\" if $opt_v;\n-\tmy $pool=SVN::Pool->new;\n-\t$svn->{'svn'}->get_log(\"/\",$from_rev,$to_rev,0,1,1,\\&commit_all,$pool);\n-\t$pool->clear;\n+\t$svn->{'svn'}->get_log(\"/\",$from_rev,$to_rev,0,1,1,\\&commit_all);\n \tmy $pid = fork();\n \tdie \"Fork: $!\\n\" unless defined $pid;\n \tunless($pid) {\n\n-- \nStefan Sperling <stsp@elego.de>                 Software Developer\nelego Software Solutions GmbH                            HRB 77719\nGustav-Meyer-Allee 25, Gebaeude 12        Tel:  +49 30 23 45 86 96 \n13355 Berlin                              Fax:  +49 30 23 45 86 95\nhttp://www.elego.de                 Geschaeftsfuehrer: Olaf Wagner\n"},{"id":"54019","messageId":"7vr6km6354.fsf@gitster.siamese.dyndns.org","threadId":"9994","inReplyTo":"20070924105740.GB8900@ted","subject":"Re: [PATCH] resend: really plug memory leaks in git-svnimport","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-09-25T17:55:35Z","receivedAt":"2007-09-25T17:55:35Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Stefan Sperling <stsp@elego.de> writes:\n\n> Junio asked me to resend this patch to the mailing list.\n>\n> This version of the patch is adjusted to apply cleanly\n> to current HEAD.\n>\n> @Junio: I'm not resending the multiple branch/tag dirs patch\n> just yet, because I want to polish it first -- I've got another\n> idea how to improve it.\n\nOk.\n\nPeople on the list who still use git-svnimport, could you help\nwith testing this patch?  Will queue for 'pu' in the meantime.\n"},{"id":"54020","messageId":"46F95580.1050907@op5.se","threadId":"9994","inReplyTo":"7vr6km6354.fsf@gitster.siamese.dyndns.org","subject":"Re: [PATCH] resend: really plug memory leaks in git-svnimport","fromName":"Andreas Ericsson","fromEmail":"ae@op5.se","sentAt":"2007-09-25T18:37:52Z","receivedAt":"2007-09-25T18:37:52Z","isPatch":true,"sender":{"key":"ae@op5.se","avatar":"https://gravatar.com/avatar/426e89595c75a8f5252dd0c989e5fabe5bcac616e68557427ad9aef6b0ca342a?d=mp&s=160"},"body":"Junio C Hamano wrote:\n> Stefan Sperling <stsp@elego.de> writes:\n> \n>> Junio asked me to resend this patch to the mailing list.\n>>\n>> This version of the patch is adjusted to apply cleanly\n>> to current HEAD.\n>>\n>> @Junio: I'm not resending the multiple branch/tag dirs patch\n>> just yet, because I want to polish it first -- I've got another\n>> idea how to improve it.\n> \n> Ok.\n> \n> People on the list who still use git-svnimport, could you help\n> with testing this patch?  Will queue for 'pu' in the meantime.\n> \n\nI used to use it, but I've given up on it in favour of git svn.\nPartly because git-svn seems to get cases right that git-svnimport\ndidn't, but mostly because it remembers where I fetched from, which\nis damn handy.\n\n-- \nAndreas Ericsson                   andreas.ericsson@op5.se\nOP5 AB                             www.op5.se\nTel: +46 8-230225                  Fax: +46 8-230231\n"}]}