Re: [PATCH 2/2] gitweb: Optimize paging when sorted by path

[Jakub Narebski <jnareb@gmail.com>]

Lubomir Rintel <lkundrak@v3.sk> writes:

> There's no need to get authors, description and last modification time
> of a project that's not being shown on a current page. We can only tell
> that in advance if the list is sorted by pathname.

More advanced version of this trick can be found in

  [RFC/PATCH] gitweb: Paginate project list
  http://thread.gmane.org/gmane.comp.version-control.git/80896/focus=81613

from 2008-05-10.  Perhaps it would be a good idea to merge these two
patches?

> ---
>  gitweb/gitweb.perl |   47 ++++++++++++++++++++++++++++++++++-------------
>  1 files changed, 34 insertions(+), 13 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index 8dc7f29..a2e9a95 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -4608,12 +4608,30 @@ sub format_sort_th {
>  	return $sort_th;
>  }
>  
> +sub git_try_to_order {
> +	my ($projects, $order) = @_;
> +
> +	my %order_info = (
> +		project => { key => 'path', type => 'str' },
> +		descr => { key => 'descr_long', type => 'str' },
> +		owner => { key => 'owner', type => 'str' },
> +		age => { key => 'age', type => 'num' }
> +	);
> +	my $oi = $order_info{$order};
> +	return undef unless exists $projects->[0]->{$oi->{'key'}};

Nitpick: there is no need to use '->' between [0] and {$oi->{'key'}}.
BTW is this sanity check necessary?

> +	if ($oi->{'type'} eq 'str') {
> +		@$projects = sort {$a->{$oi->{'key'}} cmp $b->{$oi->{'key'}}} @$projects;
> +	} else {
> +		@$projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @$projects;
> +	}
> +	return 1;
> +}

Note that separating this part of code into subroutine is a good idea
anyway, even if the final patch would look different.

> +
>  sub git_project_list_body {
>  	# actually uses global variable $project
>  	my ($projlist, $order, $from, $to, $extra, $no_header) = @_;
>  
>  	my $check_forks = gitweb_check_feature('forks');
> -	my @projects = fill_project_list_info($projlist, $check_forks);
>  
>  	$order ||= $default_projects_order;
>  	$page ||= 0;
> @@ -4622,26 +4640,29 @@ sub git_project_list_body {
>  		$to = $from + $projects_per_page - 1 unless defined $to;
>  	}
>  	$from = 0 unless defined $from;
> -	$to = $#projects if (!defined $to || $#projects < $to);
> +	$to = $#$projlist if (!defined $to || $#$projlist < $to);
>  
>  	my $prev_link = $cgi->a({-href => href(-replay=>1, page=>$page-1),
>  		 -accesskey => "p", -title => "Alt-p"}, "prev") if ($page > 0);
>  	my $next_link = $cgi->a({-href => href(-replay=>1, page=>$page+1),
>  		 -accesskey => "n", -title => "Alt-n"}, "next") if ($#$projlist > $to);
>  
> -	my %order_info = (
> -		project => { key => 'path', type => 'str' },
> -		descr => { key => 'descr_long', type => 'str' },
> -		owner => { key => 'owner', type => 'str' },
> -		age => { key => 'age', type => 'num' }
> -	);
> -	my $oi = $order_info{$order};
> -	if ($oi->{'type'} eq 'str') {
> -		@projects = sort {$a->{$oi->{'key'}} cmp $b->{$oi->{'key'}}} @projects;
> -	} else {
> -		@projects = sort {$a->{$oi->{'key'}} <=> $b->{$oi->{'key'}}} @projects;
> +	# If we're paginating and can order the list now (by pathname), we
> +	# don't need to do an unnecessary and expensive query of the details
> +	# of the projects we're not going to display. Attempt the sort and
> +	# remove the other projects from the list if the sort is successful.
> +	# Can't be used with ctags, since it needs a complete project list.
> +	my $ordered = git_try_to_order($projlist, $order)
> +		unless gitweb_check_feature('ctags');
> +	if ($ordered) {
> +		@$projlist = @$projlist[$from..$to];
> +		$to -= $from;
> +		$from = 0;
>  	}
>  
> +	my @projects = fill_project_list_info($projlist, $check_forks);
> +	git_try_to_order(\@projects, $order) unless $ordered;
> +
>  	my $show_ctags = gitweb_check_feature('ctags');
>  	if ($show_ctags) {
>  		my %ctags;
> -- 
> 1.7.2.1
> 

-- 
Jakub Narebski
Poland
ShadeHawk on #git