On Sun, 19 Oct 2008, Giuseppe Bilotta wrote:
> The @snapshot_fmts array, containing the list of the supported snapshot

quoted text
> formats, was recreated locally in three different routines (with the

> different name @supported_fmts in one of them).

>

> Its local generation is particularly expensive because two of the

> callers, href() and format_snapshot_links(), are often called many times

> in a single page.

>

> Simplify code and speed up page generation by making the array global.

>

> Signed-off-by: Giuseppe Bilotta 

Very good idea, although I'd prefer for this patch to come first;

it is not controversial contrary to other patches in this (sub)series

which IMHO needs some thought first.
Acked-by: Jakub Narebski 
> ---

quoted text
>  gitweb/gitweb.perl |   21 ++++++++-------------

>  1 files changed, 8 insertions(+), 13 deletions(-)

>

> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl

> index 5fd5a1f..d1475b7 100755

> --- a/gitweb/gitweb.perl

> +++ b/gitweb/gitweb.perl

> @@ -748,6 +748,10 @@ if (defined $searchtext) {

>  our $git_dir;

>  $git_dir = "$projectroot/$project" if $project;

>

> +# list of supported snapshot formats

> +our @snapshot_fmts = gitweb_check_feature('snapshot');

> +@snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts);

> +

>  # dispatch

>  if (!defined $action) {

>  	if (defined $hash) {

> @@ -858,11 +862,7 @@ sub href (%) {

>  			# snapshot_format should always be defined when href()

>  			# is called, but just in case some code forgets, we

>  			# fall back to the default

> -			if (!$fmt) {

> -				my @snapshot_fmts = gitweb_check_feature('snapshot');

> -				@snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts);

> -				$fmt = $snapshot_fmts[0];

> -			}

> +			$fmt ||= $snapshot_fmts[0];

>  			$href .= $known_snapshot_formats{$fmt}{'suffix'};

>  			delete $params{'snapshot_format'};

>  		}

> @@ -1695,8 +1695,6 @@ sub format_diff_line {

>  # linked.  Pass the hash of the tree/commit to snapshot.

>  sub format_snapshot_links {

>  	my ($hash) = @_;

> -	my @snapshot_fmts = gitweb_check_feature('snapshot');

> -	@snapshot_fmts = filter_snapshot_fmts(@snapshot_fmts);

>  	my $num_fmts = @snapshot_fmts;

>  	if ($num_fmts > 1) {

>  		# A parenthesized list of links bearing format names.

> @@ -4898,20 +4896,17 @@ sub git_tree {

>  }

>

>  sub git_snapshot {

> -	my @supported_fmts = gitweb_check_feature('snapshot');

> -	@supported_fmts = filter_snapshot_fmts(@supported_fmts);

> -

>  	my $format = $input_params{'snapshot_format'};

> -	if (!@supported_fmts) {

> +	if (!@snapshot_fmts) {

>  		die_error(403, "Snapshots not allowed");

>  	}

>  	# default to first supported snapshot format

> -	$format ||= $supported_fmts[0];

> +	$format ||= $snapshot_fmts[0];

>  	if ($format !~ m/^[a-z0-9]+$/) {

>  		die_error(400, "Invalid snapshot format parameter");

>  	} elsif (!exists($known_snapshot_formats{$format})) {

>  		die_error(400, "Unknown snapshot format");

> -	} elsif (!grep($_ eq $format, @supported_fmts)) {

> +	} elsif (!grep($_ eq $format, @snapshot_fmts)) {

>  		die_error(403, "Unsupported snapshot format");

>  	}

>

> --

> 1.5.6.5

>

> 

--

Jakub Narebski

Poland

--

To unsubscribe from this list: send the line "unsubscribe git" in

the body of a message to majordomo@vger.kernel.org

More majordomo info at  http://vger.kernel.org/majordomo-info.html