{"thread":{"id":"35706","subject":"[PATCH v2] improve git svn performance","startedAt":"2014-01-22T08:08:23Z","lastAt":"2014-01-22T20:25:01Z","messageCount":2,"participants":["manjian2006@gmail.com","Eric Wong"],"isPatch":true,"patchVersion":2,"patchTotal":null},"messages":[{"id":"233501","messageId":"1390378103-24392-1-git-send-email-manjian2006@gmail.com","threadId":"35706","inReplyTo":null,"subject":"[PATCH v2] improve git svn performance","fromName":"","fromEmail":"manjian2006@gmail.com","sentAt":"2014-01-22T08:08:23Z","receivedAt":"2014-01-22T08:08:23Z","isPatch":true,"sender":{"key":"manjian2006@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5935381?v=4"},"body":"From: manjian2006 \n\n\n* perl/Git/SVN.pm\n Modified according to Eric Wong \n\n>Hi, I'm interested in this. How much did performance improve by\n>(and how many revisions is the repository)>\nOur svn server are built in a LAN,15152 revisions.Not optimized git-svn used 10 hours or more to accomplish,\nwhile optimized one using only 3-4 hours.\n\n\nAccording to some profiling data,_rev_list subroutine and rebuild subroutine are consuming a large proportion of time.\nSo I improve _rev_list's performance by memoize its results,and avoid subprocess invocation by memoize rebuild subroutine's key data.\n\nSigned-off-by: manjian2006 \n---\n perl/Git/SVN.pm | 41 ++++++++++++++++++++++++++++++++++++++---\n 1 file changed, 38 insertions(+), 3 deletions(-)\n\ndiff --git a/perl/Git/SVN.pm b/perl/Git/SVN.pm\nindex 5273ee8..dc7942b 100644\n--- a/perl/Git/SVN.pm\n+++ b/perl/Git/SVN.pm\n@@ -1599,6 +1599,7 @@ sub tie_for_persistent_memoization {\n \t\tmy %lookup_svn_merge_cache;\n \t\tmy %check_cherry_pick_cache;\n \t\tmy %has_no_changes_cache;\n+\t\tmy %_rev_list_cache;\n \n \t\ttie_for_persistent_memoization(\\%lookup_svn_merge_cache,\n \t\t \"$cache_path/lookup_svn_merge\");\n@@ -1620,6 +1621,14 @@ sub tie_for_persistent_memoization {\n \t\t\tSCALAR_CACHE => ['HASH' => \\%has_no_changes_cache],\n \t\t\tLIST_CACHE => 'FAULT',\n \t\t;\n+\n+\t\ttie_for_persistent_memoization(\\%_rev_list_cache,\n+\t\t \"$cache_path/_rev_list\");\n+\t\tmemoize '_rev_list',\n+\t\t\tSCALAR_CACHE => 'FAULT',\n+\t\t\tLIST_CACHE => ['HASH' => \\%_rev_list_cache],\n+\t\t;\n+\n \t}\n \n \tsub unmemoize_svn_mergeinfo_functions {\n@@ -1629,6 +1638,7 @@ sub tie_for_persistent_memoization {\n \t\tMemoize::unmemoize 'lookup_svn_merge';\n \t\tMemoize::unmemoize 'check_cherry_pick';\n \t\tMemoize::unmemoize 'has_no_changes';\n+\t\tMemoize::unmemoize '_rev_list';\n \t}\n \n \tsub clear_memoized_mergeinfo_caches {\n@@ -1959,11 +1969,25 @@ sub rebuild_from_rev_db {\n \tunlink $path or croak \"unlink: $!\";\n }\n \n+#define a global associate map to record rebuild status\n+my %rebuild_status;\n+#define a global associate map to record rebuild verify status\n+my %rebuild_verify_status;\n+\n sub rebuild {\n \tmy ($self) = @_;\n \tmy $map_path = $self->map_path;\n \tmy $partial = (-e $map_path && ! -z $map_path);\n-\treturn unless ::verify_ref($self->refname.'^0');\n+\tmy $verify_key = $self->refname.'^0';\n+\tif (! exists $rebuild_verify_status{$verify_key} || ! defined $rebuild_verify_status{$verify_key} ) {\n+\t\tmy $verify_result = ::verify_ref($verify_key);\n+\t\tif ($verify_result) {\n+\t\t\t$rebuild_verify_status{$verify_key} = 1;\n+\t\t}\n+\t}\n+\tif (! exists $rebuild_verify_status{$verify_key}) {\n+\t\treturn;\n+\t}\n \tif (!$partial && ($self->use_svm_props || $self->no_metadata)) {\n \t\tmy $rev_db = $self->rev_db_path;\n \t\t$self->rebuild_from_rev_db($rev_db);\n@@ -1977,10 +2001,21 @@ sub rebuild {\n \tprint \"Rebuilding $map_path ...\\n\" if (!$partial);\n \tmy ($base_rev, $head) = ($partial ? $self->rev_map_max_norebuild(1) :\n \t\t(undef, undef));\n+\tmy $key_value = ($head ? \"$head..\" : \"\") . $self->refname;\n+\tif (exists $rebuild_status{$key_value}) {\n+\t\tprint \"Done rebuilding $map_path\\n\" if (!$partial || !$head);\n+\t\tmy $rev_db_path = $self->rev_db_path;\n+\t\tif (-f $self->rev_db_path) {\n+\t\t\tunlink $self->rev_db_path or croak \"unlink: $!\";\n+\t\t}\n+\t\t$self->unlink_rev_db_symlink;\n+\t\treturn;\n+\t}\n \tmy ($log, $ctx) =\n-\t command_output_pipe(qw/rev-list --pretty=raw --reverse/,\n-\t\t\t\t($head ? \"$head..\" : \"\") . $self->refname,\n+\t\tcommand_output_pipe(qw/rev-list --pretty=raw --reverse/,\n+\t\t\t\t$key_value,\t\n \t\t\t\t'--');\n+\t$rebuild_status{$key_value} = 1;\n \tmy $metadata_url = $self->metadata_url;\n \tremove_username($metadata_url);\n \tmy $svn_uuid = $self->rewrite_uuid || $self->ra_uuid;\n-- \n1.8.3.2\n"},{"id":"233539","messageId":"20140122202501.GA31356@dcvr.yhbt.net","threadId":"35706","inReplyTo":"1390378103-24392-1-git-send-email-manjian2006@gmail.com","subject":"Re: [PATCH v2] improve git svn performance","fromName":"Eric Wong","fromEmail":"normalperson@yhbt.net","sentAt":"2014-01-22T20:25:01Z","receivedAt":"2014-01-22T20:25:01Z","isPatch":true,"sender":{"key":"e@80x24.org","avatar":null},"body":"manjian2006@gmail.com wrote:\n> * perl/Git/SVN.pm\n> Modified according to Eric Wong \n> \n> >Hi, I'm interested in this. How much did performance improve by\n> >(and how many revisions is the repository)>\n\n> Our svn server are built in a LAN,15152 revisions.Not optimized\n> git-svn used 10 hours or more to accomplish, while optimized one using\n> only 3-4 hours.\n> \n> According to some profiling data,_rev_list subroutine and rebuild\n> subroutine are consuming a large proportion of time. So I improve\n> _rev_list's performance by memoize its results,and avoid subprocess\n> invocation by memoize rebuild subroutine's key data.\n\nImpressive! Thanks for that info.\n\n> Signed-off-by: manjian2006 \n\nReal name is preferred by this project, I think.\n\nA proper patch would start something like this:\n-------------------------------8<------------------------------------\nFrom: Your Name \nSubject: git-svn: memoize _rev_list and rebuild\n\nAccording to profile data, _rev_list and rebuild consume a large\nportion of time. Memoize the results of _rev_list and memoize\nrebuild internals to avoid subprocess invocation.\n\nWhen importing 15152 revisions on a LAN, time improved from 10\nhours to 3-4 hours.\n\nSigned-off-by: Your Name \n---------------------- a few more comments below -------------------\n\n> sub rebuild {\n> \tmy ($self) = @_;\n> \tmy $map_path = $self->map_path;\n> \tmy $partial = (-e $map_path && ! -z $map_path);\n> -\treturn unless ::verify_ref($self->refname.'^0');\n> +\tmy $verify_key = $self->refname.'^0';\n> +\tif (! exists $rebuild_verify_status{$verify_key} || ! defined $rebuild_verify_status{$verify_key} ) {\n\n80 column wrap, please.\n\nHowever, I think just having a single\n\"!$rebuild_verify_status{$verify_key}\" check is enough, no need for\nextra defined/exists checks for %rebuild_verify_status nor %rebuild_status.\nNeither of them load untrusted data.\n\n> -\t command_output_pipe(qw/rev-list --pretty=raw --reverse/,\n> -\t\t\t\t($head ? \"$head..\" : \"\") . $self->refname,\n> +\t\tcommand_output_pipe(qw/rev-list --pretty=raw --reverse/,\n> +\t\t\t\t$key_value,\t\n\nPlease do not leave trailing whitespace. Thanks.\n"}]}