[RESEND] Pagination for gitweb

[RESEND] Pagination for gitweb

[Lubomir Rintel]

I tought something like this could be a starter for better handling long
gitweb project lists (such as http://pkgs.fedoraproject.org/gitweb/).

Could anyone please take a look?
Thanks!

[PATCH 1/2] gitweb: Make it possible to paginate projects

[Lubomir Rintel]

This adds simple pagination (next and prev links), to project lists,
analogous to what is done for commit history lists.
---
 gitweb/gitweb.perl |   26 ++++++++++++++++++++++++++
 1 files changed, 26 insertions(+), 0 deletions(-)

diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
index a85e2f6..8dc7f29 100755
--- a/gitweb/gitweb.perl
+++ b/gitweb/gitweb.perl
@@ -255,6 +255,9 @@ our %highlight_ext = (
 	map { $_ => 'xml' } qw(xhtml html htm),
 );
 
+# Set this to non-zero to enable project list pagination
+our $projects_per_page = 0;
+
 # You define site-wide feature defaults here; override them with
 # $GITWEB_CONFIG as necessary.
 our %feature = (
@@ -4613,9 +4616,19 @@ sub git_project_list_body {
 	my @projects = fill_project_list_info($projlist, $check_forks);
 
 	$order ||= $default_projects_order;
+	$page ||= 0;
+	if ($projects_per_page) {
+		$from = $page * $projects_per_page unless defined $from;
+		$to = $from + $projects_per_page - 1 unless defined $to;
+	}
 	$from = 0 unless defined $from;
 	$to = $#projects if (!defined $to || $#projects < $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' },
@@ -4709,6 +4722,19 @@ sub git_project_list_body {
 		print "<td colspan=\"5\">$extra</td>\n" .
 		      "</tr>\n";
 	}
+
+	if ($prev_link or $next_link) {
+		print "<tr>\n";
+		if ($check_forks) {
+			print "<td></td>\n";
+		}
+		print "<td colspan=\"5\">";
+		print $prev_link if $prev_link;
+		print " &sdot; " if $prev_link and $next_link;
+		print $next_link if $next_link;
+		print "</td>\n</tr>\n";
+	}
+
 	print "</table>\n";
 }
 
-- 
1.7.2.1

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

[Lubomir Rintel]

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.
---
 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'}};
+	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;
+}
+
 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

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

[Jakub Narebski]

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

Re: [PATCH 1/2] gitweb: Make it possible to paginate projects

[Jakub Narebski]

Lubomir Rintel <lkundrak@v3.sk> writes:

> This adds simple pagination (next and prev links), to project lists,
> analogous to what is done for commit history lists.

Lack signoff (see Documentation/SubmittingPatches).

> ---
>  gitweb/gitweb.perl |   26 ++++++++++++++++++++++++++
>  1 files changed, 26 insertions(+), 0 deletions(-)
> 
> diff --git a/gitweb/gitweb.perl b/gitweb/gitweb.perl
> index a85e2f6..8dc7f29 100755
> --- a/gitweb/gitweb.perl
> +++ b/gitweb/gitweb.perl
> @@ -255,6 +255,9 @@ our %highlight_ext = (
>  	map { $_ => 'xml' } qw(xhtml html htm),
>  );
>  
> +# Set this to non-zero to enable project list pagination
> +our $projects_per_page = 0;
> +

Good idea of being able to enable or disable pagination of projects
list.  

I am not sure though if this is the correct solution.  First,
pagination in all other places is hardcoded to 100 items per page;
IMHO for consistency it would be good to use the same page size
everywhere.

Second, perhaps instead of yet another global variable a better
solution would be non-verridable %feature, like 'pathinfo' or 'forks'
features?

>  # You define site-wide feature defaults here; override them with
>  # $GITWEB_CONFIG as necessary.
>  our %feature = (
> @@ -4613,9 +4616,19 @@ sub git_project_list_body {
>  	my @projects = fill_project_list_info($projlist, $check_forks);
>  
>  	$order ||= $default_projects_order;
> +	$page ||= 0;
> +	if ($projects_per_page) {
> +		$from = $page * $projects_per_page unless defined $from;
> +		$to = $from + $projects_per_page - 1 unless defined $to;
> +	}

Hmmm...

>  	$from = 0 unless defined $from;
>  	$to = $#projects if (!defined $to || $#projects < $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);
> +

In other places we have 'first' &sdot; 'prev' &sdot; 'next'...

>  	my %order_info = (
>  		project => { key => 'path', type => 'str' },
>  		descr => { key => 'descr_long', type => 'str' },
> @@ -4709,6 +4722,19 @@ sub git_project_list_body {
>  		print "<td colspan=\"5\">$extra</td>\n" .
>  		      "</tr>\n";
>  	}
> +
> +	if ($prev_link or $next_link) {
> +		print "<tr>\n";
> +		if ($check_forks) {
> +			print "<td></td>\n";
> +		}
> +		print "<td colspan=\"5\">";
> +		print $prev_link if $prev_link;
> +		print " &sdot; " if $prev_link and $next_link;
> +		print $next_link if $next_link;
> +		print "</td>\n</tr>\n";
> +	}

... is there a reason to not use format_paging_nav() subroutine?

> +
>  	print "</table>\n";
>  }
>  
> -- 
> 1.7.2.1
> 

See also comments to next patch in series

-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [RESEND] Pagination for gitweb

[Jakub Narebski]

Lubomir Rintel <lkundrak@v3.sk> writes:

> I thought something like this could be a starter for better handling long
> gitweb project lists (such as http://pkgs.fedoraproject.org/gitweb/).
> 
> Could anyone please take a look?

What do you mean here by "better handling"?  

Is the problem server performance for large number of projects?  If
this is the problem, perhaps better solution would be to use caching
(work in progress).

Is the problem large projects-list page and bandwidth?  There was a
patch adding transparent compression of pages generated by gitweb
would be a better solution; perhaps this together with caching (to
avoid performance hit on CPU; note that usually gitweb performance is
I/O and not CPU-bound).

Is the problem client rendering performance on large page with large
table?  If it is, then paginating output, or adding project search
like in gitweb fork used on http://repo.or.cz is correct solution.


So which is it?
-- 
Jakub Narebski
Poland
ShadeHawk on #git

Re: [RESEND] Pagination for gitweb

[J.H.]

On 09/10/2010 11:57 AM, Jakub Narebski wrote:
> Lubomir Rintel <lkundrak@v3.sk> writes:
> 
>> I thought something like this could be a starter for better handling long
>> gitweb project lists (such as http://pkgs.fedoraproject.org/gitweb/).
>>
>> Could anyone please take a look?
> 
> What do you mean here by "better handling"?  
> 
> Is the problem server performance for large number of projects?  If
> this is the problem, perhaps better solution would be to use caching
> (work in progress).

They already moved to using my caching layer, mainly because I could
create an RPM for them and the fact that my caching code is slightly
more battle tested.

> Is the problem large projects-list page and bandwidth?  There was a
> patch adding transparent compression of pages generated by gitweb
> would be a better solution; perhaps this together with caching (to
> avoid performance hit on CPU; note that usually gitweb performance is
> I/O and not CPU-bound).
> 
> Is the problem client rendering performance on large page with large
> table?  If it is, then paginating output, or adding project search
> like in gitweb fork used on http://repo.or.cz is correct solution.
> 
> 
> So which is it?

I think the issue is just having something like 10K projects all
suddenly staring you in the face on a single page, it's not so much a
technical problem with gitweb itself as a mental problem of dealing with
a giant webpage like that.

So far the Fedora guys seem happy with the way things are (well except
for the fact that their front page takes something like 2 hours to
build, which has it's own set of problems).

Just some additional food for thought.

- John 'Warthog9' Hawley

Re: [RESEND] Pagination for gitweb

[Jakub Narebski]

On Fri, 10 Sep 2010, J.H. wrote:
> On 09/10/2010 11:57 AM, Jakub Narebski wrote:
> > Lubomir Rintel <lkundrak@v3.sk> writes:
> > 
> > > I thought something like this could be a starter for better handling long
> > > gitweb project lists (such as http://pkgs.fedoraproject.org/gitweb/).
> > >
> > > Could anyone please take a look?
> > 
> > What do you mean here by "better handling"?  
> > 
> > Is the problem server performance for large number of projects?  If
> > this is the problem, perhaps better solution would be to use caching
> > (work in progress).
> 
> They already moved to using my caching layer, mainly because I could
> create an RPM for them and the fact that my caching code is slightly
> more battle tested.

Note that with project list pagination, and especially with project
search  feature, a better solution might be to cache *data* rather
than HTML output, like in fork used (or used to be used) by
http://repo.or.cz, or in Lea Wiemann's GSoC 2008 project [1]

[1] http://repo.or.cz/w/git/gitweb-caching.git
-- 
Jakub Narebski
Poland

Re: [RESEND] Pagination for gitweb

[Jakub Narebski]

On Fri, 10 Sep 2010, J.H. wrote:
> On 09/10/2010 11:57 AM, Jakub Narebski wrote:
 
> > Is the problem server performance for large number of projects?  If
> > this is the problem, perhaps better solution would be to use caching
> > (work in progress).
> 
> They already moved to using my caching layer, mainly because I could
> create an RPM for them and the fact that my caching code is slightly
> more battle tested.

Is this RPM site-specific, or generic one?  Could you share *.spec file?
Or do you use git.spec from git, just use patched gitweb?
-- 
Jakub Narebski
Poland