On Fri, 2018-08-31 at 15:09 +0100, Stephen Finucane wrote:
> On Fri, 2018-08-10 at 18:00 +1000, Stewart Smith wrote:
> > e.g. a 10 comment patch goes from 26 queries in 17-20ms down to 20
> > queries in 12ms.
> >
> > A 67 comment cover letter goes from 14 queries in 16ms down to 8 queries
> > in 8ms.
> >
> > So, effectively, a near 2x perf improvement.
> >
> > Previously, at several points we were asking for the latest series and
> > then asking for all the series. Since there just usually aren't *that*
> > many series, fetch them all and take the first one if we need to.
> >
> > Signed-off-by: Stewart Smith <stew...@linux.ibm.com>
>
> Another "don't do stuff like this in templates" example. This one also
> makes sense to me and definitely improves performance.
>
> Reviewed-by: Stephen Finucane <step...@that.guru>
There's a small issue with this one also. As with the other patch, I
can fix at merge time.
> Stephen
>
> > ---
> > patchwork/templates/patchwork/submission.html | 10 +++++-----
> > patchwork/views/cover.py | 2 +-
> > patchwork/views/patch.py | 1 +
> > 3 files changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/patchwork/templates/patchwork/submission.html
> > b/patchwork/templates/patchwork/submission.html
> > index 2f69735d6925..3b6f9fbe909e 100644
> > --- a/patchwork/templates/patchwork/submission.html
> > +++ b/patchwork/templates/patchwork/submission.html
> > @@ -64,15 +64,15 @@ function toggle_div(link_id, headers_id)
> > </div>
> > </td>
> > </tr>
> > -{% if submission.latest_series %}
> > +{% if submission.all_series %}
This should presumably read 'all_series' - not 'submission.all_series'.
> > <tr>
> > <th>Series</th>
> > <td>
> > <div class="patchrelations">
> > <ul>
> > - {% for series in submission.series.all %}
> > + {% for series in all_series %}
> > <li>
> > - {% if series == submission.latest_series %}
> > + {% if forloop.first %}
> > {{ series }}
> > {% else %}
> > <a href="{% url 'patch-list' project_id=project.linkname
> > %}?series={{ series.id }}">
> > @@ -93,7 +93,7 @@ function toggle_div(link_id, headers_id)
> > >show</a>
> > <div id="patchrelations" class="patchrelations" style="display:none;">
> > <ul>
> > - {% with submission.latest_series.cover_letter as cover %}
> > + {% with all_series.cover_letter as cover %}
'all_series' is a queryset so it doesn't have a 'cover_letter'
attribute. What we want is something like this.
<div id="patchrelations" class="patchrelations" style="display:none;">
+ {% for series in all_series %}
<ul>
+ {% with series.cover_letter as cover %}
This will actually fix a small theoretical issue we have, whereby
dependencies for other series than the first weren't listed. I say
theoretical as we never actually assign more than one series to a patch
and I am actually looking at making the series-patch relationship a 1:N
relationship shortly.
Note that I didn't spot either of these initially as the templates
don't error out on missing attributes. I should check to see if that
behaviour is configurable.
Stephen
> > <li>
> > {% if cover %}
> > {% if cover == submission %}
> > @@ -106,7 +106,7 @@ function toggle_div(link_id, headers_id)
> > {% endif %}
> > </li>
> > {% endwith %}
> > - {% for sibling in submission.latest_series.patches.all %}
> > + {% for sibling in all_series.patches.all %}
> > <li>
> > {% if sibling == submission %}
> > {{ sibling.name|default:"[no subject]"|truncatechars:100 }}
> > diff --git a/patchwork/views/cover.py b/patchwork/views/cover.py
> > index edad90bc694d..1ee2b3f988fa 100644
> > --- a/patchwork/views/cover.py
> > +++ b/patchwork/views/cover.py
> > @@ -49,7 +49,7 @@ def cover_detail(request, cover_id):
> > comments = comments.select_related('submitter')
> > comments =
> > comments.only('submitter','date','id','content','submission')
> > context['comments'] = comments
> > -
> > + context['all_series'] = cover.series.all().order_by('-date')
> > return render_to_response('patchwork/submission.html', context)
> >
> >
> > diff --git a/patchwork/views/patch.py b/patchwork/views/patch.py
> > index f43fbecd9a4d..e1d0cdcfcf39 100644
> > --- a/patchwork/views/patch.py
> > +++ b/patchwork/views/patch.py
> > @@ -118,6 +118,7 @@ def patch_detail(request, patch_id):
> > comments = comments.select_related('submitter')
> > comments =
> > comments.only('submitter','date','id','content','submission')
> >
> > + context['all_series'] = patch.series.all().order_by('-date')
> > context['comments'] = comments
> > context['submission'] = patch
> > context['patchform'] = form
>
>
> _______________________________________________
> Patchwork mailing list
> Patchwork@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/patchwork
_______________________________________________
Patchwork mailing list
Patchwork@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/patchwork
