[PATCH 02/11] 4x performance improvement for viewing patch with many comments

Stephen Finucane stephen at that.guru
Sat Sep 1 00:06:08 AEST 2018


On Fri, 2018-08-10 at 18:00 +1000, Stewart Smith wrote:
> Using the example of id:20180720035941.6844-1-khandual at linux.vnet.ibm.com
> with my test dataset of a chunk of a variety of mailing lists, has
> this cover letter have 67 comments from a variety of people. Thus,
> it's on the larger side of things.
> 
> Originally, displaying the /patch/550/ for this (redirected to /cover)
> would take 81 SQL queries in ~60ms on my laptop.
> 
> After this optimisation, it's down to 14 queries in 14ms.
> 
> When the cache is cold, it's down to 32ms from 83ms.
> 
> The effect of this patch is to execute a join in the database to
> get the submitter information for each comment at the same time as
> getting all the comments rather than doing a one-by-one lookup after
> the fact.
> 
> Signed-off-by: Stewart Smith <stewart at linux.ibm.com>

Looks good to me. Daniel's already pointed out the comma-space thing
(blame pep8) but we'll fix at merge time.

Reviewed-by: Stephen Finucane <stephen at that.guru>

I think the learning here, and for other, related patches in the
series, is that we should be more careful using any model relationships
in templates.

Stephen

> ---
>  patchwork/templates/patchwork/submission.html | 2 +-
>  patchwork/views/cover.py                      | 5 +++++
>  patchwork/views/patch.py                      | 5 +++++
>  3 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/patchwork/templates/patchwork/submission.html b/patchwork/templates/patchwork/submission.html
> index e817713f7079..2f69735d6925 100644
> --- a/patchwork/templates/patchwork/submission.html
> +++ b/patchwork/templates/patchwork/submission.html
> @@ -262,7 +262,7 @@ function toggle_div(link_id, headers_id)
>  </pre>
>  </div>
>  
> -{% for item in submission.comments.all %}
> +{% for item in comments %}
>  {% if forloop.first %}
>  <h2>Comments</h2>
>  {% endif %}
> diff --git a/patchwork/views/cover.py b/patchwork/views/cover.py
> index 73f83cb99b99..edad90bc694d 100644
> --- a/patchwork/views/cover.py
> +++ b/patchwork/views/cover.py
> @@ -45,6 +45,11 @@ def cover_detail(request, cover_id):
>          'project': cover.project,
>      }
>  
> +    comments = cover.comments.all()
> +    comments = comments.select_related('submitter')
> +    comments = comments.only('submitter','date','id','content','submission')
> +    context['comments'] = comments
> +
>      return render_to_response('patchwork/submission.html', context)
>  
>  
> diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
> index cbd4ec395d99..f43fbecd9a4d 100644
> --- a/patchwork/views/patch.py
> +++ b/patchwork/views/patch.py
> @@ -114,6 +114,11 @@ def patch_detail(request, patch_id):
>      if is_authenticated(request.user):
>          context['bundles'] = request.user.bundles.all()
>  
> +    comments = patch.comments.all()
> +    comments = comments.select_related('submitter')
> +    comments = comments.only('submitter','date','id','content','submission')
> +
> +    context['comments'] = comments
>      context['submission'] = patch
>      context['patchform'] = form
>      context['createbundleform'] = createbundleform




More information about the Patchwork mailing list