From 07b3dffd2014f7796f1940e0fafe5f12fabb59fe Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 7 Mar 2012 22:20:15 +0100 Subject: [PATCH] Reduce I/O in build listings by only fetching required columns Columns such as "longDescription" can be large, so fetching them when they're not needed is wasteful. --- src/lib/Hydra/Base/Controller/ListBuilds.pm | 1 + src/lib/Hydra/Base/Controller/NixChannel.pm | 7 +++--- src/lib/Hydra/Controller/Job.pm | 2 +- src/lib/Hydra/Controller/Jobset.pm | 2 +- src/lib/Hydra/Controller/Root.pm | 2 +- src/lib/Hydra/Helper/CatalystUtils.pm | 24 +++++++++++++++------ 6 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/lib/Hydra/Base/Controller/ListBuilds.pm b/src/lib/Hydra/Base/Controller/ListBuilds.pm index 1419d5ad..81524cf5 100644 --- a/src/lib/Hydra/Base/Controller/ListBuilds.pm +++ b/src/lib/Hydra/Base/Controller/ListBuilds.pm @@ -65,6 +65,7 @@ sub all : Chained('get_builds') PathPart { $c->stash->{builds} = [ $c->stash->{allBuilds}->search( { finished => 1 }, { order_by => "timestamp DESC" + , columns => [@buildListColumns] , rows => $resultsPerPage , page => $page }) ]; } diff --git a/src/lib/Hydra/Base/Controller/NixChannel.pm b/src/lib/Hydra/Base/Controller/NixChannel.pm index 1d28735a..ec5ba60d 100644 --- a/src/lib/Hydra/Base/Controller/NixChannel.pm +++ b/src/lib/Hydra/Base/Controller/NixChannel.pm @@ -51,16 +51,15 @@ sub nixexprs : Chained('nix') PathPart('nixexprs.tar.bz2') Args(0) { sub name { my ($build) = @_; - return $build->get_column('releasename') || $build->nixname; + return $build->releasename || $build->nixname; } sub sortPkgs { - # Sort by name, then timestamp. + # Sort by name, then id. return sort { lc(name($a->{build})) cmp lc(name($b->{build})) - or $a->{build}->timestamp <=> $b->{build}->timestamp - } @_; + or $a->{build}->id <=> $b->{build}->id } @_; } diff --git a/src/lib/Hydra/Controller/Job.pm b/src/lib/Hydra/Controller/Job.pm index 536dada2..2f442629 100644 --- a/src/lib/Hydra/Controller/Job.pm +++ b/src/lib/Hydra/Controller/Job.pm @@ -29,7 +29,7 @@ sub overview : Chained('job') PathPart('') Args(0) { $c->stash->{lastBuilds} = [ $c->stash->{job}->builds->search({ finished => 1 }, - { order_by => 'timestamp DESC', rows => 10 }) ]; + { order_by => 'timestamp DESC', rows => 10, columns => [@buildListColumns] }) ]; $c->stash->{runningBuilds} = [ $c->stash->{job}->builds->search( diff --git a/src/lib/Hydra/Controller/Jobset.pm b/src/lib/Hydra/Controller/Jobset.pm index e5e5cb00..d9a9b2a9 100644 --- a/src/lib/Hydra/Controller/Jobset.pm +++ b/src/lib/Hydra/Controller/Jobset.pm @@ -83,7 +83,7 @@ sub jobsetIndex { # Last builds for jobset. $c->stash->{lastBuilds} = [ $c->stash->{jobset}->builds->search({ finished => 1 }, - { order_by => "timestamp DESC", rows => 5 }) ]; + { order_by => "timestamp DESC", rows => 5, columns => [@buildListColumns] }) ]; } diff --git a/src/lib/Hydra/Controller/Root.pm b/src/lib/Hydra/Controller/Root.pm index 79a02d18..5e2ff399 100644 --- a/src/lib/Hydra/Controller/Root.pm +++ b/src/lib/Hydra/Controller/Root.pm @@ -74,7 +74,7 @@ sub queue :Local { my ($self, $c) = @_; $c->stash->{template} = 'queue.tt'; $c->stash->{queue} = [$c->model('DB::Builds')->search( - {finished => 0}, {join => ['project'] , order_by => ["priority DESC", "timestamp"], '+select' => ['project.enabled'], '+as' => ['enabled'] })]; + {finished => 0}, { join => ['project'], order_by => ["priority DESC", "timestamp"], columns => [@buildListColumns], '+select' => ['project.enabled'], '+as' => ['enabled'] })]; $c->stash->{flashMsg} = $c->flash->{buildMsg}; } diff --git a/src/lib/Hydra/Helper/CatalystUtils.pm b/src/lib/Hydra/Helper/CatalystUtils.pm index d4673f3f..95f77037 100644 --- a/src/lib/Hydra/Helper/CatalystUtils.pm +++ b/src/lib/Hydra/Helper/CatalystUtils.pm @@ -13,9 +13,14 @@ our @EXPORT = qw( requireLogin requireProjectOwner requireAdmin requirePost isAdmin isProjectOwner trim $pathCompRE $relPathRE $relNameRE $jobNameRE $systemRE + @buildListColumns ); +# Columns from the Builds table needed to render build lists. +Readonly our @buildListColumns => ('id', 'finished', 'timestamp', 'project', 'jobset', 'job', 'nixname', 'system', 'priority', 'busy', 'buildstatus', 'releasename'); + + sub getBuild { my ($c, $id) = @_; my $build = $c->model('DB::Builds')->find($id); @@ -97,7 +102,9 @@ sub getBuildStats { sub getChannelData { my ($c, $builds) = @_; - my @builds2 = $builds->search_literal("exists (select 1 from buildproducts where build = me.id and type = 'nix-build')"); + my @builds2 = $builds + ->search_literal("exists (select 1 from buildproducts where build = me.id and type = 'nix-build')") + ->search({}, { columns => [@buildListColumns, 'drvpath', 'outpath', 'description', 'homepage'] }); my @storePaths = (); foreach my $build (@builds2) { @@ -144,12 +151,14 @@ sub requireLogin { $c->detach; # doesn't return } + sub isProjectOwner { my ($c, $project) = @_; return $c->user_exists && ($c->check_user_roles('admin') || $c->user->username eq $project->owner->username || defined $c->model('DB::ProjectMembers')->find({ project => $project, userName => $c->user->username })); } + sub requireProjectOwner { my ($c, $project) = @_; @@ -166,6 +175,7 @@ sub isAdmin { return $c->user_exists && $c->check_user_roles('admin'); } + sub requireAdmin { my ($c) = @_; @@ -190,12 +200,12 @@ sub trim { # Security checking of filenames. -Readonly::Scalar our $pathCompRE => "(?:[A-Za-z0-9-\+\._][A-Za-z0-9-\+\._]*)"; -Readonly::Scalar our $relPathRE => "(?:$pathCompRE(?:/$pathCompRE)*)"; -Readonly::Scalar our $relNameRE => "(?:[A-Za-z0-9-][A-Za-z0-9-\.]*)"; -Readonly::Scalar our $attrNameRE => "(?:[A-Za-z_][A-Za-z0-9_]*)"; -Readonly::Scalar our $jobNameRE => "(?:$attrNameRE(?:\\.$attrNameRE)*)"; -Readonly::Scalar our $systemRE => "(?:[a-z0-9_]+-[a-z0-9_]+)"; +Readonly our $pathCompRE => "(?:[A-Za-z0-9-\+\._][A-Za-z0-9-\+\._]*)"; +Readonly our $relPathRE => "(?:$pathCompRE(?:/$pathCompRE)*)"; +Readonly our $relNameRE => "(?:[A-Za-z0-9-][A-Za-z0-9-\.]*)"; +Readonly our $attrNameRE => "(?:[A-Za-z_][A-Za-z0-9_]*)"; +Readonly our $jobNameRE => "(?:$attrNameRE(?:\\.$attrNameRE)*)"; +Readonly our $systemRE => "(?:[a-z0-9_]+-[a-z0-9_]+)"; 1;