launchpad-reviewers team mailing list archive

["Jeroen T. Vermeulen" <jtv@xxxxxxxxxxxxx>]

Jeroen T. Vermeulen has proposed merging lp:~jtv/launchpad/templates-listing into lp:launchpad/devel.

Requested reviews:
  Launchpad code reviewers (launchpad-reviewers): code
Related bugs:
  #608349 Timeouts on DistroSeries:+templates page
  https://bugs.launchpad.net/bugs/608349


= Bug 608349 =

Even after the phenomenal optimization job Adi and Danilo did on the DistroSeries:+templates lists, Zope and TAL are still taking too much time to make this page render comfortably.  In the bug ticket they identify further optimization opportunities: avoiding the URL formatter and replacing the templating engine with a faster one.

In this hobby branch I move the rendering of the template listing rows from the TAL into the browser code.  This keeps the TAL cleaner and simpler, takes the TAL parser out of the critical loop, and allows for more conventional optimization.  I hope the new browser code in its own way exposes the structure more clearly than the TAL with its conditionals and irregularly-sized columns did.

I did keep the central loop in TAL.  Growing one huge string for potentially thousands of templates could get quite unwieldy; the templating engine will hopefully do things in a more "streaming" fashion.  With any luck the listing's HTML can start flowing to the client before rendering has completed.

Two "conventional optimizations" are noteworthy:
 * The view classes can (and do) provide their own, faster alternatives to canonical_url.
 * Using self.user instead of context/required:launchpad.AnyPerson avoids the Zope security infrastructure.

By far most of the new code is actually in a new unit-test for the views.  The browser-code approach allows for much more extensive and detailed testing.  In particular of course the test verifies that the optimized URL generation code is equivalent to canonical_url.  It actually produces shorter, relative URLs; this is the only semantic change and it affected two of our existing test stories in a very minor way.  The rest of the HTML also got shorter and I think easier to read.


Jeroen
-- 
https://code.launchpad.net/~jtv/launchpad/templates-listing/+merge/35185
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~jtv/launchpad/templates-listing into lp:launchpad/devel.