#12639 closed enhancement (fixed)
Integrate Jinja2 branch — at Version 7
| Reported by: | Christian Boos | Owned by: | Christian Boos |
|---|---|---|---|
| Priority: | high | Milestone: | 1.3.2 |
| Component: | rendering | Version: | 1.3dev |
| Severity: | critical | Keywords: | jinja2 performance +testing |
| Cc: | Branch: | ||
| Release Notes: |
The template engine used by Trac is now Jinja2. Genshi templates rendering in plugins will be supported until Trac 1.5.1. |
||
| API Changes: |
Major changes in See details in TracDev/ReleaseNotes/1.3#Developer-visiblechanges. |
||
| Internal Changes: | |||
Description
The work on the switch to the Jinja2 engine is mostly complete on the experimental branch cboos.git@jinja2.
Like explained in TracDev/Proposals/Jinja, this change will adequately solve the performance issues we had with Genshi: pages render from 3x to 10x faster, at a fraction of the cost in memory.
This ticket is about the finalization and review, and the merge of the changes on trunk.
Besides the merge and catching up with the changes that happened on trunk since the merge-base (r14499), the main open points are:
- get rid of the j* prefix used for the Jinja2 pages and find another mechanism to support both Jinja2 templates and legacy Genshi templates
- test l10n for template pages in plugins (no domain directive)
Change History (8)
comment:1 by , 7 years ago
comment:2 by , 7 years ago
Sure, the 'j' prefix is just a hack that allowed me to have both series of templates in the same folder while making the conversion. It's not supposed to stay.
comment:3 by , 7 years ago
Quick overview of the Python code churn, as it stands (related to #12130):
$ git diff trunk...jinja2 --stat | grep ".py" contrib/jinjachecker.py | 433 +++++++ contrib/jinjastats.py | 63 + doc/conf.py | 1 + sample-plugins/HelloWorld.py | 6 +- sample-plugins/Timestamp.py | 2 +- sample-plugins/revision_links.py | 3 +- sample-plugins/workflow/CodeReview.py | 3 +- sample-plugins/workflow/StatusFixer.py | 5 +- sample-plugins/workflow/VoteOperation.py | 3 +- setup.py | 29 +- trac/about.py | 8 +- trac/admin/tests/functional.py | 20 +- trac/admin/web_ui.py | 4 +- trac/attachment.py | 3 +- trac/config.py | 3 +- trac/core.py | 2 +- trac/db/api.py | 2 +- trac/db/mysql_backend.py | 3 +- trac/db/postgres_backend.py | 3 +- trac/db/sqlite_backend.py | 3 +- trac/db/tests/api.py | 2 +- trac/dist.py | 229 +++- trac/env.py | 10 +- trac/mimeview/api.py | 105 +- trac/mimeview/patch.py | 6 +- trac/mimeview/pygments.py | 58 +- trac/mimeview/rst.py | 4 +- trac/mimeview/templates/jprefs_pygments.html | 72 ++ trac/mimeview/tests/api.py | 113 +- trac/mimeview/tests/functional.py | 4 +- trac/mimeview/tests/patch.py | 24 +- trac/mimeview/tests/pygments.data | 91 ++ trac/mimeview/tests/pygments.html | 57 - trac/mimeview/tests/pygments.py | 51 +- trac/notification/compat.py | 20 +- trac/notification/mail.py | 3 +- trac/notification/prefs.py | 5 +- trac/notification/tests/mail.py | 3 +- trac/prefs/tests/functional.py | 2 +- trac/prefs/web_ui.py | 24 +- trac/resource.py | 2 +- trac/search/web_ui.py | 9 +- trac/tests/compat.py | 12 +- trac/tests/functional/better_twill.py | 1 + trac/tests/functional/testcases.py | 6 +- trac/tests/functional/tester.py | 2 +- trac/tests/resource.py | 2 +- trac/ticket/api.py | 3 +- trac/ticket/batch.py | 3 +- trac/ticket/default_workflow.py | 3 +- trac/ticket/notification.py | 21 +- trac/ticket/query.py | 17 +- trac/ticket/report.py | 3 +- trac/ticket/roadmap.py | 8 +- trac/ticket/tests/conversion.py | 8 +- trac/ticket/tests/default_workflow.py | 4 +- trac/ticket/tests/functional/admin.py | 69 +- trac/ticket/tests/functional/default_workflow.py | 40 +- trac/ticket/tests/functional/main.py | 168 +-- trac/ticket/tests/query.py | 87 +- trac/ticket/tests/web_ui.py | 14 +- trac/ticket/web_ui.py | 19 +- trac/timeline/tests/web_ui.py | 6 +- trac/timeline/web_ui.py | 24 +- trac/util/html.py | 905 ++++++++++++-- trac/util/presentation.py | 225 +++- trac/util/tests/__init__.py | 4 +- trac/util/tests/html.py | 254 ++-- trac/util/tests/presentation.py | 17 + trac/util/text.py | 48 + trac/util/translation.py | 21 +- trac/versioncontrol/admin.py | 12 +- trac/versioncontrol/diff.py | 5 +- trac/versioncontrol/tests/functional.py | 10 +- trac/versioncontrol/web_ui/browser.py | 6 +- trac/versioncontrol/web_ui/changeset.py | 19 +- trac/versioncontrol/web_ui/log.py | 4 +- trac/versioncontrol/web_ui/util.py | 3 +- trac/web/__init__.py | 19 +- trac/web/api.py | 35 +- trac/web/auth.py | 16 +- trac/web/chrome.py | 657 +++++++--- trac/web/main.py | 36 +- trac/web/tests/api.py | 19 +- trac/web/tests/chrome.py | 2 +- trac/wiki/api.py | 3 +- trac/wiki/formatter.py | 43 +- trac/wiki/intertrac.py | 4 +- trac/wiki/interwiki.py | 3 +- trac/wiki/macros.py | 5 +- trac/wiki/tests/formatter.py | 35 +- trac/wiki/tests/functional.py | 41 +- trac/wiki/tests/macros.py | 44 +- trac/wiki/web_ui.py | 14 +- tracopt/ticket/clone.py | 61 +- tracopt/ticket/commit_updater.py | 3 +- tracopt/ticket/deleter.py | 73 +- tracopt/versioncontrol/git/git_fs.py | 4 +- tracopt/versioncontrol/svn/svn_prop.py | 3 +- tracopt/versioncontrol/svn/tests/svn_fs.py | 148 ++-
follow-up: 5 comment:4 by , 7 years ago
| Owner: | set to |
|---|---|
| Status: | new → assigned |
I've completed the rebase on latest trunk.
Here's a commented break down of the changesets, to ease reviewing. The changesets are listed oldest first.
Note that I started by squashing all the changes, then created smaller commits according to functional units. While I rebased on trunk, I also added some bug fixes.
Here's the initial infrastructure for the switch to Jinja2:
- [eefd6476/cboos.git] (#12639) add jinja2 dependency
- [362758ea/cboos.git] (#12639) i18n support
- [dac78330/cboos.git] (#12639) CI support
- [a6e98398/cboos.git] (#12639) tooling (jinjachecker)
Quite a few utility functions and classes were added,
the lowest level is in trac.util.text (Jinja2 being
primarily a text based template engine), then trac.util.html
and finally trac.util.presentation:
- [d942caa8/cboos.git] (#12639) text utils
- [846ba2d4/cboos.git] (#12639) html utils
- [aaa1b03b/cboos.git] (#12639) presentation utils
- [b09015a7/cboos.git] (#12639) i18n utils
The integration of Jinja2 then continues in the higher-level common layers of Trac, the wiki formatter and the mimeview API:
- [c972bd1b/cboos.git] (#12639) adapt wiki formatter
- [ff952273/cboos.git] (#12639) adapt mimeview API
- [37dae6ac/cboos.git] (#12639) adapt mimeview patch API
- [28e85999/cboos.git] (#12639) adapt mimeview pygments API
- [5df8972c/cboos.git] (#12639) adapt mimeview rST and tests
All the old direct Genshi imports are converted to the new
(backward compatible) ones from trac.util.html:
- [c944ccf0/cboos.git] (#12639) adapt to new html utils
Now the big piece, enabling the Jinja2 template to be loaded, populated with data and generated:
- [5684a60c/cboos.git] (#12639) adapt chrome API
- [0cefa267/cboos.git] (#12639) adapt web API
- [b0478dea/cboos.git] (#12639) adapt web auth
- [61193e5a/cboos.git] (#12639) adapt to new chrome API
At this point, we can start to work on the templates proper:
- [864d088b/cboos.git] (#12639) Genshi templates changes
- [cfb715fd/cboos.git] (#12639) generic jinja2 templates
- [19e1025f/cboos.git] (#12639) integrate trunk changes on generic templates
- [75c8be3f/cboos.git] (#12639) misc CSS changes
Now we start the conversion module by module, with the occasional commit reflecting the changes on trunk since… last year:
- [df3f2175/cboos.git] (#12639) adapt functional tests infrastructure
- [1d3b22af/cboos.git] (#12639) adapt about module
- [eb95aac4/cboos.git] (#12639) adapt admin module
- [0f6bc07b/cboos.git] (#12639) adapt attachment module
- [33c32fae/cboos.git] (#12639) adapt prefs module
- [4a460408/cboos.git] (#12639) adapt search module
- [df6cf9b8/cboos.git] (#12639) adapt ticket module
- [f490fc17/cboos.git] (#12639, #10735) adapt ticket module
- [0a4917e4/cboos.git] (#12639) integrate trunk changes on ticket templates
- [9f87c9e1/cboos.git] (#12639) adapt timeline module
- [071f6c5b/cboos.git] (#12639) adapt versioncontrol module
- [864ecaad/cboos.git] (#12639) adapt versioncontrol svn backend
- [8f3278c6/cboos.git] (#12639) adapt wiki tests
- [62709d2b/cboos.git] (#12639) adapt wiki module
So far we have just a refresh of the original Jinja2 branch, with all the back and forth eliminated.
As I did the merge, I worked on finding a way to be able to render indifferently Jinja2 and legacy Genshi templates, without resorting to the "j" prefixing hack that was convenient during the migration period.
This resulted in some quite important API changes, the main trick being that the "old" API is still supported but if used, it is an indication that Genshi templates should be used. For using Jinja2 templates, the newer API should be used. All the details in the following log message of the following commits:
- [62452bdf/cboos.git] (#12639): web and chrome API changes for Jinja2 vs. Genshi
- [7ccbfb16/cboos.git] (#12639) fix ticket web_ui test (XPath replacement)
- [6709a8db/cboos.git] (#12639): final chrome API changes for Jinja2
That's it, thanks for having followed so far, and please give it a shot. The integration is really overdue, I've been using the branch successfully for the past 6 months without major issues, and it's time to spread to good news ;-)
I'm particularly interested in feedback on the last two important API changes commits, as it's quite new and I suspect there are still a few improvements possible.
Another area which still needs work is i18n, as I haven't looked closely there since a while. TODO
Finally, Jinja 2.9 is out (or will be out soon), and it comes with a bug fix I've been waiting for all along, so I plan to adapt to that version and make it a minimal requirement. TODO
$ git diff --stat trunk jinja2-trunk-r15341 ... 214 files changed, 15353 insertions(+), 1877 deletions(-)
Well, when we'll remove most of the Genshi templates, that will also give some more deletions!
comment:5 by , 7 years ago
Replying to Christian Boos:
Finally, Jinja 2.9 is out
It's out and already used for the AppVeyor and Travis builds. I guess the errors in the functional tests seen there must be due to this new version, as I've not tested 2.9 myself yet.
comment:6 by , 7 years ago
| Keywords: | +testing added |
|---|
The branch can now be seen in action in testing.
For example, this are the timings for 00faab14 with 30 lines of context:
| testing | t.e.o | |
| Waiting (TTFB) | 9.65 s | 27.25 s |
| Content Download | 409.63 ms | 598.59 ms |
(measured with the network panel in Chrome)
At my first try, I didn't see much speed difference between the two, so I increased the context to 30 lines, hoping the difference would be more noticeable. It was, as I got a MemoryError as a result! Then I realized we have the SpamFilter installed there, and one of its filter uses the ITemplateStreamFilter interface. Once I disabled that tracspamfilter.filters.trapfield.* component, the results were again in line with the expectations (i.e. what you see in the table above).
So I hope that this will be a compelling argument for people to upgrade their plugins and get rid of the ITemplateStreamFilter interface support. As soon as you have one active, you lose the performance improvements gained with the switch to Jinja2. And the hack for supporting the stream filters seems to be quite memory hungry.
by , 7 years ago
| Attachment: | spamfilter-jinja2-compat-r15349.diff added |
|---|
make the SpamFilter compatible with the jinja2-trunk-r.. branch after [0f3aee46/cboos.git]
comment:7 by , 7 years ago
| API Changes: | modified (diff) |
|---|---|
| Release Notes: | modified (diff) |
| Resolution: | → fixed |
| Status: | assigned → closed |
Branch landed on trunk today [15406-15474].
Notes: 2 plugins on trac-hacks.org are using template named with
j*.