Re: [PATHv2 8/8] gitweb: make the supported snapshot formats array global
- From
Jakub Narebski <jnareb@gmail.com>
- Date
- Nov 2, 2008, 01:54 UTC
- Message-ID
- <200811020255.01895.jnareb@gmail.com>
- In-Reply-To
- <1224426270-27755-3-git-send-email-giuseppe.bilotta@gmail.com>
On Sun, 19 Oct 2008, Giuseppe Bilotta wrote:
Show 11 quoted lines
> 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@gmail.com>
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@gmail.com>
Show 69 quoted lines
> ---
> 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