git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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
Previous: Giuseppe BilottaNext: Junio C Hamano
Message 11 of 26 in “[PATCHv6 0/5] gitweb: PATH_INFO enhancement”
  1. Giuseppe BilottaOct 16, 2008
  2. 1/5 gitweb: parse project/action/hash_base:filename PATH_INFOGiuseppe Bilotta, Oct 16, 2008
  3. 2/5 gitweb: generate project/action/hash URLsGiuseppe Bilotta, Oct 16, 2008
  4. 3/5 gitweb: use_pathinfo filenames start with /Giuseppe Bilotta, Oct 16, 2008
  5. 4/5 gitweb: parse parent..current syntax from PATH_INFOGiuseppe Bilotta, Oct 16, 2008
  6. 5/5 gitweb: generate parent..current URLsGiuseppe Bilotta, Oct 16, 2008
  7. 6/6 gitweb: retrieve snapshot format from PATH_INFOGiuseppe Bilotta, Oct 19, 2008
  8. 6/8 gitweb: retrieve snapshot format from PATH_INFOGiuseppe Bilotta, Oct 19, 2008
  9. 7/8 gitweb: embed snapshot format parameter in PATH_INFOGiuseppe Bilotta, Oct 19, 2008
  10. 8/8 gitweb: make the supported snapshot formats array globalGiuseppe Bilotta, Oct 19, 2008
  11. Jakub NarebskiNov 2, 2008
  12. Junio C HamanoNov 2, 2008
  13. Jakub NarebskiNov 1, 2008
  14. Giuseppe BilottaNov 1, 2008
  15. Jakub NarebskiOct 21, 2008
  16. Giuseppe BilottaOct 21, 2008
  17. Junio C HamanoOct 21, 2008
  18. Jakub NarebskiOct 20, 2008
  19. Giuseppe BilottaOct 20, 2008
  20. Jakub NarebskiOct 20, 2008
  21. Giuseppe BilottaOct 20, 2008
  22. Jakub NarebskiOct 18, 2008
  23. Giuseppe BilottaOct 18, 2008
  24. Jakub NarebskiOct 19, 2008
  25. Jakub NarebskiOct 18, 2008
  26. Jakub NarebskiOct 18, 2008

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.