{"thread":{"id":"8126","subject":"[PATCH 1/2] git-svn: clean up caching of SVN::Ra functions","startedAt":"2007-05-13T08:04:43Z","lastAt":"2007-05-13T18:17:44Z","messageCount":4,"participants":["Eric Wong","Pierre Habouzit","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"42002","messageId":"11790434841909-git-send-email-normalperson@yhbt.net","threadId":"8126","inReplyTo":null,"subject":"[PATCH 1/2] git-svn: clean up caching of SVN::Ra functions","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2007-05-13T08:04:43Z","receivedAt":"2007-05-13T08:04:43Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"This patch was originally intended to make the Perl GC more\nsensitive to the SVN::Pool objects and not accidentally clean\nthem up when they shouldn't be (causing segfaults).  That didn't\nwork, but this patch makes the code a bit cleaner regardless\n\nPut our caches for get_dir and check_path calls directly into\nthe SVN::Ra object so they auto-expire when it is destroyed.\n\ndirents returned by get_dir() no longer needs the pool object\nstored persistently along with the cache data, as they'll be\nconverted to native Perl hash references.\n\nSince calling rev_proplist repeatedly per-revision is no longer\nneeded in git-svn, we do not cache calls to it.\n\nSigned-off-by: Eric Wong <normalperson@yhbt.net>\n---\n git-svn.perl |   68 +++++++++++++++++++++++++++++++++++----------------------\n 1 files changed, 42 insertions(+), 26 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex 2e80d41..ee69598 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -1390,7 +1390,7 @@ sub traverse_ignore {\n \t\t}\n \t}\n \tforeach (sort keys %$dirent) {\n-\t\tnext if $dirent->{$_}->kind != $SVN::Node::dir;\n+\t\tnext if $dirent->{$_}->{kind} != $SVN::Node::dir;\n \t\t$self->traverse_ignore($fh, \"$path/$_\", $r);\n \t}\n }\n@@ -2888,7 +2888,7 @@ my ($can_do_switch, %ignored_err, $RA);\n BEGIN {\n \t# enforce temporary pool usage for some simple functions\n \tmy $e;\n-\tforeach (qw/get_latest_revnum get_uuid get_repos_root/) {\n+\tforeach (qw/rev_proplist get_latest_revnum get_uuid get_repos_root/) {\n \t\t$e .= \"sub $_ {\n \t\t\tmy \\$self = shift;\n \t\t\tmy \\$pool = SVN::Pool->new;\n@@ -2897,29 +2897,7 @@ BEGIN {\n \t\t\twantarray ? \\@ret : \\$ret[0]; }\\n\";\n \t}\n \n-\t# get_dir needs $pool held in cache for dirents to work,\n-\t# check_path is cacheable and rev_proplist is close enough\n-\t# for our purposes.\n-\tforeach (qw/check_path get_dir rev_proplist/) {\n-\t\t$e .= \"my \\%${_}_cache; my \\$${_}_rev = 0; sub $_ {\n-\t\t\tmy \\$self = shift;\n-\t\t\tmy \\$r = pop;\n-\t\t\tmy \\$k = join(\\\"\\\\0\\\", \\@_);\n-\t\t\tif (my \\$x = \\$${_}_cache{\\$r}->{\\$k}) {\n-\t\t\t\treturn wantarray ? \\@\\$x : \\$x->[0];\n-\t\t\t}\n-\t\t\tmy \\$pool = SVN::Pool->new;\n-\t\t\tmy \\@ret = \\$self->SUPER::$_(\\@_, \\$r, \\$pool);\n-\t\t\tif (\\$r != \\$${_}_rev) {\n-\t\t\t\t\\%${_}_cache = ( pool => [] );\n-\t\t\t\t\\$${_}_rev = \\$r;\n-\t\t\t}\n-\t\t\t\\$${_}_cache{\\$r}->{\\$k} = \\\\\\@ret;\n-\t\t\tpush \\@{\\$${_}_cache{pool}}, \\$pool;\n-\t\t\twantarray ? \\@ret : \\$ret[0]; }\\n\";\n-\t}\n-\t$e .= \"\\n1;\";\n-\teval $e or die $@;\n+\teval \"$e; 1;\" or die $@;\n }\n \n sub new {\n@@ -2952,9 +2930,47 @@ sub new {\n \t$self->{svn_path} = $url;\n \t$self->{repos_root} = $self->get_repos_root;\n \t$self->{svn_path} =~ s#^\\Q$self->{repos_root}\\E(/|$)##;\n+\t$self->{cache} = { check_path => { r => 0, data => {} },\n+\t                   get_dir => { r => 0, data => {} } };\n \t$RA = bless $self, $class;\n }\n \n+sub check_path {\n+\tmy ($self, $path, $r) = @_;\n+\tmy $cache = $self->{cache}->{check_path};\n+\tif ($r == $cache->{r} && exists $cache->{data}->{$path}) {\n+\t\treturn $cache->{data}->{$path};\n+\t}\n+\tmy $pool = SVN::Pool->new;\n+\tmy $t = $self->SUPER::check_path($path, $r, $pool);\n+\t$pool->clear;\n+\tif ($r != $cache->{r}) {\n+\t\t%{$cache->{data}} = ();\n+\t\t$cache->{r} = $r;\n+\t}\n+\t$cache->{data}->{$path} = $t;\n+}\n+\n+sub get_dir {\n+\tmy ($self, $dir, $r) = @_;\n+\tmy $cache = $self->{cache}->{get_dir};\n+\tif ($r == $cache->{r}) {\n+\t\tif (my $x = $cache->{data}->{$dir}) {\n+\t\t\treturn wantarray ? @$x : $x->[0];\n+\t\t}\n+\t}\n+\tmy $pool = SVN::Pool->new;\n+\tmy ($d, undef, $props) = $self->SUPER::get_dir($dir, $r, $pool);\n+\tmy %dirents = map { $_ => { kind => $d->{$_}->kind } } keys %$d;\n+\t$pool->clear;\n+\tif ($r != $cache->{r}) {\n+\t\t%{$cache->{data}} = ();\n+\t\t$cache->{r} = $r;\n+\t}\n+\t$cache->{data}->{$dir} = [ \\%dirents, $r, $props ];\n+\twantarray ? (\\%dirents, $r, $props) : \\%dirents;\n+}\n+\n sub DESTROY {\n \t# do not call the real DESTROY since we store ourselves in $RA\n }\n@@ -3175,7 +3191,7 @@ sub match_globs {\n \t\treturn unless scalar @x == 3;\n \t\tmy $dirents = $x[0];\n \t\tforeach my $de (keys %$dirents) {\n-\t\t\tnext if $dirents->{$de}->kind != $SVN::Node::dir;\n+\t\t\tnext if $dirents->{$de}->{kind} != $SVN::Node::dir;\n \t\t\tmy $p = $g->{path}->full_path($de);\n \t\t\tnext if $exists->{$p};\n \t\t\tnext if (length $g->{path}->{right} &&\n-- \n1.5.2.rc3.18.gf0c86\n"},{"id":"42003","messageId":"11790434862131-git-send-email-normalperson@yhbt.net","threadId":"8126","inReplyTo":"11790434841909-git-send-email-normalperson@yhbt.net","subject":"[PATCH 2/2] git-svn: fix segfaults due to initial SVN pool being cleared","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2007-05-13T08:04:44Z","receivedAt":"2007-05-13T08:04:44Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"Some parts of SVN always seem to use it, even if the SVN::Ra\nobject we're using is no longer used and we've created a new one\nin its place.  It's also true that only one SVN::Ra connection\ncan exist at once...  Using SVN::Pool->new_default when the\nSVN::Ra object is created doesn't seem to help very much,\neither...\n\nHopefully this fixes all segfault problems users have been\nexperiencing over the past few months.\n\nSigned-off-by: Eric Wong <normalperson@yhbt.net>\n---\n git-svn.perl |    1 -\n 1 files changed, 0 insertions(+), 1 deletions(-)\n\ndiff --git a/git-svn.perl b/git-svn.perl\nindex ee69598..5352470 100755\n--- a/git-svn.perl\n+++ b/git-svn.perl\n@@ -2904,7 +2904,6 @@ sub new {\n \tmy ($class, $url) = @_;\n \t$url =~ s!/+$!!;\n \treturn $RA if ($RA && $RA->{url} eq $url);\n-\t$RA->{pool}->clear if $RA;\n \n \tSVN::_Core::svn_config_ensure($config_dir, undef);\n \tmy ($baton, $callbacks) = SVN::Core::auth_open_helper([\n-- \n1.5.2.rc3.18.gf0c86\n"},{"id":"42028","messageId":"20070513150530.GA29082@artemis","threadId":"8126","inReplyTo":"11790434862131-git-send-email-normalperson@yhbt.net","subject":"Re: [PATCH 2/2] git-svn: fix segfaults due to initial SVN pool being cleared","fromName":"Pierre Habouzit","fromEmail":"madcoder@debian.org","sentAt":"2007-05-13T15:05:30Z","receivedAt":"2007-05-13T15:05:30Z","isPatch":true,"sender":{"key":"madcoder@debian.org","avatar":"https://avatars.githubusercontent.com/u/44708?v=4"},"body":"On Sun, May 13, 2007 at 01:04:44AM -0700, Eric Wong wrote:\n> Some parts of SVN always seem to use it, even if the SVN::Ra\n> object we're using is no longer used and we've created a new one\n> in its place.  It's also true that only one SVN::Ra connection\n> can exist at once...  Using SVN::Pool->new_default when the\n> SVN::Ra object is created doesn't seem to help very much,\n> either...\n> \n> Hopefully this fixes all segfault problems users have been\n> experiencing over the past few months.\n> \n> Signed-off-by: Eric Wong <normalperson@yhbt.net>\n> ---\n>  git-svn.perl |    1 -\n>  1 files changed, 0 insertions(+), 1 deletions(-)\n> \n> diff --git a/git-svn.perl b/git-svn.perl\n> index ee69598..5352470 100755\n> --- a/git-svn.perl\n> +++ b/git-svn.perl\n> @@ -2904,7 +2904,6 @@ sub new {\n>  \tmy ($class, $url) = @_;\n>  \t$url =~ s!/+$!!;\n>  \treturn $RA if ($RA && $RA->{url} eq $url);\n> -\t$RA->{pool}->clear if $RA;\n>  \n>  \tSVN::_Core::svn_config_ensure($config_dir, undef);\n>  \tmy ($baton, $callbacks) = SVN::Core::auth_open_helper([\n> -- \n> 1.5.2.rc3.18.gf0c86\n\n  I confirm it fixes every segfault I was able to reproduce with any\nprior version :)\n\n-- \n·O·  Pierre Habouzit\n··O                                                madcoder@debian.org\nOOO                                                http://www.madism.org\n"},{"id":"42033","messageId":"7virawtwon.fsf@assigned-by-dhcp.cox.net","threadId":"8126","inReplyTo":"20070513150530.GA29082@artemis","subject":"Re: [PATCH 2/2] git-svn: fix segfaults due to initial SVN pool being cleared","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-05-13T18:17:44Z","receivedAt":"2007-05-13T18:17:44Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Pierre Habouzit <madcoder@debian.org> writes:\n\n>> Hopefully this fixes all segfault problems users have been\n>> experiencing over the past few months.\n>> \n>> Signed-off-by: Eric Wong <normalperson@yhbt.net>\n> ...\n>   I confirm it fixes every segfault I was able to reproduce with any\n> prior version :)\n\nThat's wonderful.  Thanks for testing, everybody.  And thanks\nfor fixing, Eric.\n"}]}