Re: [PATCH v2] improve git svn performance
- From
- Eric Wong <normalperson@yhbt.net>
- Date
- Jan 22, 2014, 20:25 UTC
- Message-ID
- <20140122202501.GA31356@dcvr.yhbt.net>
- In-Reply-To
- <1390378103-24392-1-git-send-email-manjian2006@gmail.com>
manjian2006@gmail.com wrote:
Show 5 quoted lines
> * perl/Git/SVN.pm > Modified according to Eric Wong <normalperson@yhbt.net> > > >Hi, I'm interested in this. How much did performance improve by > >(and how many revisions is the repository)>
Show 8 quoted lines
> Our svn server are built in a LAN,15152 revisions.Not optimized > git-svn used 10 hours or more to accomplish, while optimized one using > only 3-4 hours. > > According to some profiling data,_rev_list subroutine and rebuild > subroutine are consuming a large proportion of time. So I improve > _rev_list's performance by memoize its results,and avoid subprocess > invocation by memoize rebuild subroutine's key data.
Impressive! Thanks for that info.
> Signed-off-by: manjian2006 <manjian2006@gmail.com>
Real name is preferred by this project, I think.
A proper patch would start something like this: -------------------------------8<------------------------------------ From: Your Name <manjian2006@gmail.com> Subject: git-svn: memoize _rev_list and rebuild
According to profile data, _rev_list and rebuild consume a large portion of time. Memoize the results of _rev_list and memoize rebuild internals to avoid subprocess invocation.
When importing 15152 revisions on a LAN, time improved from 10 hours to 3-4 hours.
Signed-off-by: Your Name <manjian2006@gmail.com> ---------------------- a few more comments below -------------------
Show 7 quoted lines
> sub rebuild {
> my ($self) = @_;
> my $map_path = $self->map_path;
> my $partial = (-e $map_path && ! -z $map_path);
> - return unless ::verify_ref($self->refname.'^0');
> + my $verify_key = $self->refname.'^0';
> + if (! exists $rebuild_verify_status{$verify_key} || ! defined $rebuild_verify_status{$verify_key} ) {80 column wrap, please.
However, I think just having a single "!$rebuild_verify_status{$verify_key}" check is enough, no need for extra defined/exists checks for %rebuild_verify_status nor %rebuild_status. Neither of them load untrusted data.
> - command_output_pipe(qw/rev-list --pretty=raw --reverse/, > - ($head ? "$head.." : "") . $self->refname, > + command_output_pipe(qw/rev-list --pretty=raw --reverse/, > + $key_value,
Please do not leave trailing whitespace. Thanks.