Code review comment for lp:~brian-murray/launchpad/display-dupe-in-portlet-dupe-subscribers

On Thu, Sep 9, 2010 at 8:57 AM, Brian Murray <email address hidden> wrote:
> Only one additional query, per duplicate subscriber, is ran with this change.

Thats *huge*. Alarm bells should be going off in your head right now.

Bugs commonly peak at 40 or more duplicates.
Every one of those duplicates will have *at least* one subscriber (the
filer) + structural subscribptions [perhaps not impacted].

>  Looking at the bug-portlet-dupe-subscribers-content page w/o the changes 27 queries are issued and with the change 28 queries are issued for a bug with one duplicate subscriber.

Adding 1 is a boundary case: when testing, please always add 2, or 10,
to be really sure.

> No, I did not consider reworking the subscribers portlet.  Primarily, because I happened to learning about page templates and how they work and remembered a bug I'd reported and thought it'd be a quick and easy change.  However, the subscriber name is actually shortened (tal:block replace="subscription/person/fmt:displayname/fmt:shorten/20"), to make it fit in the portlet, so that might not work so well.

It seems nicer to rework the portlet to me. Anyhow, with the scaling
factor you have this change will cause performance regressions if
landed, so I'm going to set it to WIP & needs fixing.

Thanks,
Rob