[PATCH v3 00/13] send-email: various optimizations to speed up by >2x

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



A re-roll of v2[1] which fixes issues pointed out with v2, plus some
others I noticed along the way.

There are now no longer any changes to the "public"
Git::config_regxp() API. Instead it's left bitrotting in our tree at
the end of this, having 0 in-tree users (instead of the 1 currently).

We also handle the -c foo.bar (NULL value) case correctly, per what
Jeff King pointed out. I also found a related issue with
GIT_TEST_PERL_FATAL_WARNINGS=true, which we can now turn on for
git-send-email.perl.

1. https://lore.kernel.org/git/cover-00.10-00000000000-20210520T081826Z-avarab@xxxxxxxxx/

Ævar Arnfjörð Bjarmason (13):
  send-email tests: support GIT_TEST_PERL_FATAL_WARNINGS=true
  send-email tests: test for boolean variables without a value
  send-email: remove non-working support for "sendemail.smtpssl"
  send-email: refactor sendemail.smtpencryption config parsing
  send-email: copy "config_regxp" into git-send-email.perl
  send-email: lazily load config for a big speedup
  send-email: lazily shell out to "git var"
  send-email: use function syntax instead of barewords
  send-email: get rid of indirect object syntax
  send-email: lazily load modules for a big speedup
  perl: lazily load some common Git.pm setup code
  send-email: move trivial config handling to Perl
  perl: nano-optimize by replacing Cwd::cwd() with Cwd::getcwd()

 Documentation/config/sendemail.txt |   3 -
 git-send-email.perl                | 174 +++++++++++++++++++----------
 perl/Git.pm                        |  35 +++---
 t/t9001-send-email.sh              |  29 +++++
 4 files changed, 160 insertions(+), 81 deletions(-)

Range-diff against v2:
 -:  ---------- >  1:  71f890dc60 send-email tests: support GIT_TEST_PERL_FATAL_WARNINGS=true
 -:  ---------- >  2:  707c2ca556 send-email tests: test for boolean variables without a value
 1:  8474acae68 =  3:  3bbd48dab2 send-email: remove non-working support for "sendemail.smtpssl"
 2:  b87f53adbe =  4:  bed0f98d68 send-email: refactor sendemail.smtpencryption config parsing
 -:  ---------- >  5:  c12f69a411 send-email: copy "config_regxp" into git-send-email.perl
 3:  1b27a393ae !  6:  d1c233d251 send-email: lazily load config for a big speedup
    @@ git-send-email.perl: sub read_config {
      			next unless defined $v;
      			next if $configured->{$setting}++;
      			$$target = $v;
    -@@ git-send-email.perl: sub read_config {
    - 	}
    +@@ git-send-email.perl: sub config_regexp {
    + 	return @ret;
      }
      
     +# Save ourselves a lot of work of shelling out to 'git config' (it
     +# parses 'bool' etc.) by only doing so for config keys that exist.
     +my %known_config_keys;
     +{
    -+	my @known_config_keys = Git::config_regexp("^sende?mail[.]");
    ++	my @known_config_keys = config_regexp("^sende?mail[.]");
     +	@known_config_keys{@known_config_keys} = ();
     +}
     +
    @@ git-send-email.perl: sub read_config {
      my $rc = GetOptions(
      	"identity=s" => \$identity,
      	"no-identity" => \$no_identity,
    -@@ git-send-email.perl: sub read_config {
    +@@ git-send-email.perl: sub config_regexp {
      # Now we know enough to read the config
      {
          my %configured;
    @@ git-send-email.perl: sub read_config {
      }
      
      # Begin by accumulating all the variables (defined above), that we will end up
    -@@ git-send-email.perl: sub read_config {
    +@@ git-send-email.perl: sub config_regexp {
          usage();
      }
      
    --if ($forbid_sendmail_variables && (scalar Git::config_regexp("^sendmail[.]")) != 0) {
    +-if ($forbid_sendmail_variables && (scalar config_regexp("^sendmail[.]")) != 0) {
     +if ($forbid_sendmail_variables && grep { /^sendmail/s } keys %known_config_keys) {
      	die __("fatal: found configuration options for 'sendmail'\n" .
      		"git-send-email is configured with the sendemail.* options - note the 'e'.\n" .
      		"Set sendemail.forbidSendmailVariables to false to disable this check.\n");
    -
    - ## perl/Git.pm ##
    -@@ perl/Git.pm: sub config_regexp {
    - 	} catch Git::Error::Command with {
    - 		my $E = shift;
    - 		if ($E->value() == 1) {
    --			my @matches = ();
    --			return @matches;
    -+			# Key(s) not found.
    -+			return;
    - 		} else {
    - 			throw $E;
    - 		}
 4:  acee22b77d !  7:  4326c2f99c send-email: lazily shell out to "git var"
    @@ Commit message
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
     
      ## git-send-email.perl ##
    -@@ git-send-email.perl: sub read_config {
    +@@ git-send-email.perl: sub config_regexp {
      }
      
      my ($repoauthor, $repocommitter);
 5:  f317cd1c01 =  8:  e1fc71e3f9 send-email: use function syntax instead of barewords
 6:  fc27024f83 =  9:  a806ce06f1 send-email: get rid of indirect object syntax
 7:  f86f5453d7 ! 10:  aa11439789 send-email: lazily load modules for a big speedup
    @@ git-send-email.perl
     @@
      use 5.008;
      use strict;
    - use warnings;
    + use warnings $ENV{GIT_PERL_FATAL_WARNINGS} ? qw(FATAL all) : ();
     -use POSIX qw/strftime/;
     -use Term::ReadLine;
      use Getopt::Long;
    @@ git-send-email.perl: sub do_edit {
      
      	# SMTP password masked
      	system "stty echo";
    -@@ git-send-email.perl: sub read_config {
    +@@ git-send-email.perl: sub config_regexp {
      }
      
      sub parse_address_line {
 8:  86641377c0 = 11:  b3b342b173 perl: lazily load some common Git.pm setup code
 9:  895c9e29a9 ! 12:  950dc0f53d send-email: move trivial config handling to Perl
    @@ Commit message
     
         Optimize the startup time of git-send-email by using an amended
         config_regexp() function to retrieve the list of config keys and
    -    values we're interested in. See the earlier "send-email: lazily load
    -    config for a big speedup" commit for why changing its interface is OK.
    +    values we're interested in.
     
         For boolean keys we can handle the [true|false] case ourselves, and
         the "--get" case didn't need any parsing. Let's leave "--path" and
    -    other "--bool" cases to "git config". As noted in a preceding commit
    -    we're free to change the config_regexp() function, it's only used by
    -    "git send-email".
    +    other "--bool" cases to "git config". I'm not bothering with the
    +    "undef" or "" case (true and false, respectively), let's just punt on
    +    those and others and have "git config --type=bool" handle it.
     
         This brings the runtime of "git send-email" from ~60-~70ms to a very
         steady ~40ms on my test box. We now run just one "git config"
    @@ Commit message
     
         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
     
    +    diff --git a/git-send-email.perl b/git-send-email.perl
    +    index 1e9273fd4f..1ea4d9589d 100755
    +    --- a/git-send-email.perl
    +    +++ b/git-send-email.perl
    +    @@ -324,7 +324,11 @@ sub read_config {
    +                    my $target = $config_bool_settings{$setting};
    +                    my $key = "$prefix.$setting";
    +                    next unless exists $known_keys->{$key};
    +    -               my $v = Git::config_bool(@repo, $key);
    +    +               my $v = (@{$known_keys->{$key}} == 1 &&
    +    +                        (defined $known_keys->{$key}->[0] &&
    +    +                         $known_keys->{$key}->[0] =~ /^(?:true|false)$/s))
    +    +                       ? $known_keys->{$key}->[0] eq 'true'
    +    +                       : Git::config_bool(@repo, $key);
    +                    next unless defined $v;
    +                    next if $configured->{$setting}++;
    +                    $$target = $v;
    +    @@ -353,14 +357,12 @@ sub read_config {
    +                    my $key = "$prefix.$setting";
    +                    next unless exists $known_keys->{$key};
    +                    if (ref($target) eq "ARRAY") {
    +    -                       my @values = Git::config(@repo, $key);
    +    -                       next unless @values;
    +    +                       my @values = @{$known_keys->{$key}};
    +                            next if $configured->{$setting}++;
    +                            @$target = @values;
    +                    }
    +                    else {
    +    -                       my $v = Git::config(@repo, $key);
    +    -                       next unless defined $v;
    +    +                       my $v = $known_keys->{$key}->[0];
    +                            next if $configured->{$setting}++;
    +                            $$target = $v;
    +                    }
    +    @@ -371,12 +373,19 @@ sub config_regexp {
    +            my ($regex) = @_;
    +            my @ret;
    +            eval {
    +    -               @ret = Git::command(
    +    +               my $ret = Git::command(
    +                            'config',
    +    -                       '--name-only',
    +    +                       '--null',
    +                            '--get-regexp',
    +                            $regex,
    +                    );
    +    +               @ret = map {
    +    +                       # We must always return ($k, $v) here, since
    +    +                       # empty config values will be just "key\0",
    +    +                       # not "key\nvalue\0".
    +    +                       my ($k, $v) = split /\n/, $_, 2;
    +    +                       ($k, $v);
    +    +               } split /\0/, $ret;
    +                    1;
    +            } or do {
    +                    # If we have no keys we're OK, otherwise re-throw
    +    @@ -389,8 +398,10 @@ sub config_regexp {
    +     # parses 'bool' etc.) by only doing so for config keys that exist.
    +     my %known_config_keys;
    +     {
    +    -       my @known_config_keys = config_regexp("^sende?mail[.]");
    +    -       @known_config_keys{@known_config_keys} = ();
    +    +       my @kv = config_regexp("^sende?mail[.]");
    +    +       while (my ($k, $v) = splice @kv, 0, 2) {
    +    +               push @{$known_config_keys{$k}} => $v;
    +    +       }
    +     }
    +
    +     # sendemail.identity yields to --identity. We must parse this
    +
      ## git-send-email.perl ##
     @@ git-send-email.perl: sub read_config {
      		my $target = $config_bool_settings{$setting};
    @@ git-send-email.perl: sub read_config {
      		next unless exists $known_keys->{$key};
     -		my $v = Git::config_bool(@repo, $key);
     +		my $v = (@{$known_keys->{$key}} == 1 &&
    -+			 $known_keys->{$key}->[0] =~ /^(?:true|false)$/s)
    ++			 (defined $known_keys->{$key}->[0] &&
    ++			  $known_keys->{$key}->[0] =~ /^(?:true|false)$/s))
     +			? $known_keys->{$key}->[0] eq 'true'
     +			: Git::config_bool(@repo, $key);
      		next unless defined $v;
    @@ git-send-email.perl: sub read_config {
      			next if $configured->{$setting}++;
      			$$target = $v;
      		}
    -@@ git-send-email.perl: sub read_config {
    +@@ git-send-email.perl: sub config_regexp {
    + 	my ($regex) = @_;
    + 	my @ret;
    + 	eval {
    +-		@ret = Git::command(
    ++		my $ret = Git::command(
    + 			'config',
    +-			'--name-only',
    ++			'--null',
    + 			'--get-regexp',
    + 			$regex,
    + 		);
    ++		@ret = map {
    ++			# We must always return ($k, $v) here, since
    ++			# empty config values will be just "key\0",
    ++			# not "key\nvalue\0".
    ++			my ($k, $v) = split /\n/, $_, 2;
    ++			($k, $v);
    ++		} split /\0/, $ret;
    + 		1;
    + 	} or do {
    + 		# If we have no keys we're OK, otherwise re-throw
    +@@ git-send-email.perl: sub config_regexp {
      # parses 'bool' etc.) by only doing so for config keys that exist.
      my %known_config_keys;
      {
    --	my @known_config_keys = Git::config_regexp("^sende?mail[.]");
    +-	my @known_config_keys = config_regexp("^sende?mail[.]");
     -	@known_config_keys{@known_config_keys} = ();
    -+	my @kv = Git::config_regexp("^sende?mail[.]");
    ++	my @kv = config_regexp("^sende?mail[.]");
     +	while (my ($k, $v) = splice @kv, 0, 2) {
     +		push @{$known_config_keys{$k}} => $v;
     +	}
      }
      
      # sendemail.identity yields to --identity. We must parse this
    -
    - ## perl/Git.pm ##
    -@@ perl/Git.pm: sub config_int {
    - =item config_regexp ( RE )
    - 
    - Retrieve the list of configuration key names matching the regular
    --expression C<RE>. The return value is a list of strings matching
    --this regex.
    -+expression C<RE>. The return value is an ARRAY of key-value pairs.
    - 
    - =cut
    - 
    - sub config_regexp {
    - 	my ($self, $regex) = _maybe_self(@_);
    - 	try {
    --		my @cmd = ('config', '--name-only', '--get-regexp', $regex);
    -+		my @cmd = ('config', '--null', '--get-regexp', $regex);
    - 		unshift @cmd, $self if $self;
    --		my @matches = command(@cmd);
    --		return @matches;
    -+		my $data = command(@cmd);
    -+		my (@kv) = map { split /\n/, $_, 2 } split /\0/, $data;
    -+		return @kv;
    - 	} catch Git::Error::Command with {
    - 		my $E = shift;
    - 		if ($E->value() == 1) {
10:  97455f993d = 13:  c1d7ea664a perl: nano-optimize by replacing Cwd::cwd() with Cwd::getcwd()
-- 
2.32.0.rc0.406.g05cb3eebfc




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux