[email protected] wrote:
> From: linzj <[email protected]>
> I am trying to improve git svn's performance according to some
> profiling data.As the data showed,_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.Here's my patch:
Hi, I'm interested in this. How much did performance improve by
(and how many revisions is the repository)>
However, I cannot accept the patch in the current state.
The commit message is inadequate; and you need a Signed-off-by and follow
existing coding convention/style. See Documentation/SubmittingPatches
and Documentation/CodingGuidelines for more info.
Some comments inline...
> tie_for_persistent_memoization(\%lookup_svn_merge_cache,
> - "$cache_path/lookup_svn_merge");
> + "$cache_path/lookup_svn_merge");
> memoize 'lookup_svn_merge',
> - SCALAR_CACHE => 'FAULT',
> - LIST_CACHE => ['HASH' => \%lookup_svn_merge_cache],
> - ;
> + SCALAR_CACHE => 'FAULT',
> + LIST_CACHE => ['HASH' =>
> \%lookup_svn_merge_cache],
> + ;
Please no extraneous whitespace changes.
> +#define a global associate map to record rebuild status
> +my %rebuildStatus;
We prefer snake_case variables, not camelCase.
> 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 $verifyKey = $self->refname.'^0';
> + if (! exists $rebuildVerifyStatus{$verifyKey} || ! defined
> $rebuildVerifyStatus{$verifyKey} ) {
> + my $verifyResult = ::verify_ref($verifyKey);
> + if ($verifyResult) {
> + $rebuildVerifyStatus{$verifyKey} = 1;
> + }
> + }
> + if (! exists $rebuildVerifyStatus{$verifyKey}) {
> + return;
> + }
Please use tabs for indentation to match surrounding code
(most of git is tabs).
Thanks.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
