[PATCH 01/11] Improve patch listing performance (~3x)

[Stephen Finucane]

On Mon, 2018-09-10 at 23:26 +1000, Daniel Axtens wrote:
> Stephen Finucane <stephen at that.guru> writes:
> 
> > On Fri, 2018-08-10 at 18:00 +1000, Stewart Smith wrote:
> > > There's two main bits that are really expensive when composing the list
> > > of patches for a project: the query getting the list, and the query
> > > finding the series for each patch.
> > > 
> > > If we look at the query getting the list, it gets a lot of unnesseccary
> > > fields such as 'headers' and 'content', even though we tell Django not
> > > to. It turns out that Django seems to ignore the Submission relationship
> > > and I have no idea how to force it to ignore that thing (defer doesn't
> > > work) but if we go only, then it works okay.
> > > 
> > > From my import of ~8000 messages for a few projects, my laptop query
> > > time (MySQL, as setup by whatever the docker-compose things do) goes
> > > from:
> > > 
> > > http://localhost:8000/project/linux-kernel/list/
> > > FROM:
> > > 342ms SQL queries cold cache, 268ms warm cache
> > > TO:
> > > 118ms SQL queries cold cache, 88ms warm cache
> > > 
> > > Which is... non trivial to say the least.
> > > 
> > > The big jump is the patches.only change, and the removal of ordering
> > > on the patchseries takes a further 10ms off. For some strange reason, it
> > > seems rather hard to tell Django that you don't care what order the
> > > results come back in for that query (if we do, then the db server has to
> > > do a sort rather than just return each row)
> > > 
> > > Signed-off-by: Stewart Smith <stewart at linux.ibm.com>
> > 
> > Tested this and it looks good to me. There's a migration missing (it's
> > instead squashed into the first "add index" patch) but I can fix that
> > at merge time.
> > 
> > I was concerned that the combination of '.only' followed immediately by
> > '.prefetch_related' (for checks and series) would be an issue and
> > indeed it seems that, at this patch set at least, we do generate a
> > query per check. However, this is no different to before and it
> > resolved in a later check. As such:
> > 
> > Reviewed-by: Stephen Finucane <stephen at that.guru>
> > 
> > I'm not going to push this (or any of the series) yet in case Daniel
> > wants to weigh in but will do so next week if there are no complaints.
> 
> I'm not likely to find the time to do so, so if you're happy I'm happy :)
> Merge at will!

Sweet. Have pushed all of this now so.

Stephen

> Regards,
> Daniel
> 
> > 
> > Stephen
> > 
> > > ---
> > >  patchwork/models.py         | 1 -
> > >  patchwork/views/__init__.py | 2 ++
> > >  2 files changed, 2 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/patchwork/models.py b/patchwork/models.py
> > > index 6268f5b72e3c..d2b88fc48c91 100644
> > > --- a/patchwork/models.py
> > > +++ b/patchwork/models.py
> > > @@ -747,7 +747,6 @@ class Series(FilenameMixin, models.Model):
> > >          return self.name if self.name else 'Untitled series #%d' % self.id
> > >  
> > >      class Meta:
> > > -        ordering = ('date',)
> > >          verbose_name_plural = 'Series'
> > >  
> > >  
> > > diff --git a/patchwork/views/__init__.py b/patchwork/views/__init__.py
> > > index f8d23a388ac7..96fd0798af5a 100644
> > > --- a/patchwork/views/__init__.py
> > > +++ b/patchwork/views/__init__.py
> > > @@ -287,6 +287,8 @@ def generic_list(request, project, view, view_args=None, filter_settings=None,
> > >      # rendering the list template
> > >      patches = patches.select_related('state', 'submitter', 'delegate')
> > >  
> > > +    patches = patches.only('state','submitter','delegate','project','name','date')
> > > +
> > >      # we also need checks and series
> > >      patches = patches.prefetch_related('check_set', 'series')
> > >  
> > 
> > 
> > _______________________________________________
> > Patchwork mailing list
> > Patchwork at lists.ozlabs.org
> > https://lists.ozlabs.org/listinfo/patchwork