On Sun, 19 Oct 2008, Giuseppe Bilotta wrote:
> The @snapshot_fmts array, containing the list of the supported snapshot
> 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 <giuseppe.bilotta@xxxxxxxxx>
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 <jnareb@xxxxxxxxx>
> ---
> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
