Opened 11 years ago

Last modified 11 years ago

#20434 assigned New feature

Have a template tag grammar instead of handling token/parser for every tag, and make it possible to introspect the grammar.

Reported by: jonathanslenders Owned by: jonathanslenders
Component: Template system Version:
Severity: Normal Keywords:
Cc: berker.peksag@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Right now, if people use @register.simpletag (or assignment_tag/inclusion_tag), a compiled parser function will be registered in the library.tags mapping.

This however, is insufficient, if you need to do some introspection on the available template tags. I'd like to know for instance which template tags in module X are an instance of simpletag and would thus create a SimpleNode.

Right now, we are unable to accurately build a real AST from the template without manually defining which templatetags behave in which way. This is also required for AST transformations, like template compression and such.

I guess, it would be great to encourage people to always use @register.simpletag or anything that uses the built-in Node classes of Django. We can't force that, but it has a lot of advantages.

Attachments (5)

patch1.patch (35.1 KB ) - added by jonathanslenders 11 years ago.
creation of the generic TemplateTag, the Grammar, and a little refactoring to make inclusion_tag, assignment_tag and simple_tag use this one behind. It also includes all the unit tests for this.
patch2.2.patch (105.9 KB ) - added by jonathanslenders 11 years ago.
Refactoring of the built-in tags to use the TemplateTag and Grammar class. (and thus become introspectable.)
patch2.patch (105.9 KB ) - added by jonathanslenders 11 years ago.
Refactoring of the built-in tags to use the TemplateTag and Grammar class. (and thus become introspectable.)
patch3.patch (1.8 KB ) - added by jonathanslenders 11 years ago.
The templatetags management command.
patch4.patch (24.6 KB ) - added by jonathanslenders 11 years ago.
Refactoring of the tags in contrib, to use TemplateTag and Grammar.

Download all attachments as: .zip

Change History (18)

comment:1 by jonathanslenders, 11 years ago

Owner: changed from nobody to jonathanslenders
Status: newassigned

comment:2 by Jef Geskens, 11 years ago

Triage Stage: UnreviewedAccepted

Great idea!

comment:3 by jonathanslenders, 11 years ago

We should also create a new default type: @register.block_tag
for introspection and consistency in declaration of block tags.

Following will be created after registering such a block_tag.

autoescape, comment, filter, for, ifequal, ifnotequal, spaceless, with

The following as well, but allow usage of middle (else/elif) tags.

if, ifchanged

I still need to see how to fit verbatim in this grammar:

verbatim

The built-in refactored if-tag would become something like this:

@register.block_tag(tags='if elif* else? endif')
def do_if(blocks):
    for name, params, nodelist in blocks:
       ...
    return IfNode(conditions_nodelist)

Also, the simple template tags like {% now %}, {% widthratio %} etc... should be written als @register.simple_tag if possible.

Last edited 11 years ago by jonathanslenders (previous) (diff)

comment:4 by jonathanslenders, 11 years ago

Summary: Templatetag Library should keep track of which template tags are instances of simple_tag, assignment_tag and inclusion_tag.Simple_tag, assignment_tag and inclusion_tag should remember their grammar in the compiled function. Also introduce something like block_tag.

comment:5 by jonathanslenders, 11 years ago

Summary: Simple_tag, assignment_tag and inclusion_tag should remember their grammar in the compiled function. Also introduce something like block_tag.Have a template tag grammar instead of handling token/parser for every tag, and make it possible to introspect the grammar.

comment:6 by jonathanslenders, 11 years ago

After two weeks of reworking out the best way to write Django template tags, without loosing any backwards compatibility, I came to the following conclusion.
(Without the block_tag from a previous comment.)

Observation:

  • (Almost) All the template tags can be written using a grammar like: "if elif* else? endif".
  • All the template tags return a Node instance.
  • There is almost never a need to handle the parser tokens yourself. (Even in Django core. The exceptions I found were comment, extends and blocktrans.)
  • Code completion or template validation is simply impossible, because we lack any kind of introspection on the template system.
  • We DON'T want to loose backwards compatibility. Any tag which is defined the old way, using parser and token should keep working!

The generic template tags:

  • A new class TemplateTag is introduced, which inherits from Node.
  • It has a grammar attribute, which handles the parser/token and returns a parse_result instance to the constructor of TemplateTag. The grammar is probably the shipped Grammar class which accepts strings like "if endif", but can be any other external grammar library. django-classy-tags is an example of what a "perfect" grammar could look like, but that's over engineerd for django core. It is however easy to plug in this grammar

This TemplateTag implementation is very small, but it handles all the built-in templatetags in core and contrib very well. (Except a few exceptions.)

For instance:

@register.tag
class SpacelessNode(TemplateTag):
    grammar = Grammar('spaceless endspaceless')

    def render(self, context):
        from django.utils.html import strip_spaces_between_tags
        return strip_spaces_between_tags(self.nodelist.render(context).strip())

Even the if-tag looks small:

@register.tag
class IfNode(TemplateTag):
    grammar = Grammar('if elif* else? endif')

    def __init__(self, parser, parse_result):
        self.conditions_nodelists = []
        for p in parse_result.parts:
            if p.name in ('if', 'elif'):
                condition = TemplateIfParser(parser, p.arguments).parse()
                self.conditions_nodelists.append( (condition, p.nodelist) )
            elif p.name == 'else':
                self.conditions_nodelists.append( (None, p.nodelist) )

    def render(self, context):
        for condition, nodelist in self.conditions_nodelists:
            if condition is not None:           # if / elif clause
                try:
                    match = condition.eval(context)
                except VariableDoesNotExist:
                    match = None
            else:                               # else clause
                match = True

            if match:
                return nodelist.render(context)

        return ''

The attached patches are:

  • patch1: creation of the generic TemplateTag, the Grammar, and a little refactoring to make inclusion_tag, assignment_tag and simple_tag use this one behind. It also includes all the unit tests for this.
  • patch2: Refactoring of the built-in tags to use the TemplateTag and Grammar class. (and thus become introspectable.)
  • patch3: Addition of the templatetags management command, which prints an overview of the available template tags in a library and their grammar. This is necessary for external editors to have code completion. +unit tests
  • patch4: rewrite of the templatetags in contrib. Again, that's good to make more use of django in itself, but it's also required for the ability to extract the grammar rules.

I realise that some of the patches aren't really short. As a starting point I recommend to have a look at patch1 in the file django/template/generic.py

The output of the management command looks like the following. The grammar is simply extracted using introspection on the registered TemplateTags. As a parameter to the management command, you can pass a library name.

$ ./manage.py templatetags
autoescape  ::= autoescape endautoescape
block       ::= block endblock
comment     ::= comment endcomment
csrf_token  ::= csrf_token
cycle       ::= cycle
debug       ::= debug
extends     ::= extends
filter      ::= filter endfilter
firstof     ::= firstof
for         ::= for empty? endfor
if          ::= if elif* else? endif
ifchanged   ::= ifchanged else? endifchanged
ifequal     ::= ifequal else? endifequal
ifnotequal  ::= ifnotequal else? endifnotequal
include     ::= include
load        ::= load
now         ::= now
regroup     ::= regroup
spaceless   ::= spaceless endspaceless
ssi         ::= ssi
templatetag ::= templatetag
url         ::= url
verbatim    ::= verbatim endverbatim
widthratio  ::= widthratio
with        ::= with endwith

Related tickets: #7806 #9757

Cheers!
Jonathan

Last edited 11 years ago by jonathanslenders (previous) (diff)

by jonathanslenders, 11 years ago

Attachment: patch1.patch added

creation of the generic TemplateTag, the Grammar, and a little refactoring to make inclusion_tag, assignment_tag and simple_tag use this one behind. It also includes all the unit tests for this.

by jonathanslenders, 11 years ago

Attachment: patch2.2.patch added

Refactoring of the built-in tags to use the TemplateTag and Grammar class. (and thus become introspectable.)

by jonathanslenders, 11 years ago

Attachment: patch2.patch added

Refactoring of the built-in tags to use the TemplateTag and Grammar class. (and thus become introspectable.)

by jonathanslenders, 11 years ago

Attachment: patch3.patch added

The templatetags management command.

by jonathanslenders, 11 years ago

Attachment: patch4.patch added

Refactoring of the tags in contrib, to use TemplateTag and Grammar.

comment:7 by Anssi Kääriäinen, 11 years ago

This one will need some benchmarks. Check djangobench for some possible benchmarks (https://github.com/jacobian/djangobench/). You might also want to test this with other templates than the somewhat limited amount of benchmarks in djangobench. After a quick look at the patches I think this isn't going to cause any big changes in performance. But guessing performance numbers often fails, so better to actually benchmark this...

I can't say much about the patches, template engine isn't in my expertise area.

comment:8 by jonathanslenders, 11 years ago

Okay, I'll try this benchmark tool.

Normally -- unless I did something very wrong -- the start-up time could be a little higher. Parsing the templates can be a little slower because of an additional abstraction layer causing some more function calls. The rendering itself should be exactly the same. So, when people use the CachedTemplateLoader, like recommended, they shouldn't notice anything.

Of course, this needs to be verified with a tool like yours. Thanks!

comment:9 by jonathanslenders, 11 years ago

@akaariai,

I used this fork of djangobench, the latest version doesn't seem to work for django 1.5
https://github.com/charettes/djangobench/tree/django-1.5

This are the results. I'm not sure how to interpret them.

Running 'l10n_render' benchmark ...
Min: 0.004574 -> 0.005087: 1.1122x slower
Avg: 0.005782 -> 0.006625: 1.1459x slower
Significant (t=-2.226464)
Stddev: 0.00164 -> 0.00212: 1.2945x larger (N = 50)

Running 'template_compilation' benchmark ...
Min: 0.000080 -> 0.000265: 3.3095x slower
Avg: 0.000870 -> 0.000896: 1.0301x slower
Not significant
Stddev: 0.00305 -> 0.00291: 1.0499x smaller (N = 50)

Running 'template_render' benchmark ...
Min: 0.008402 -> 0.010302: 1.2261x slower
Avg: 0.009809 -> 0.014170: 1.4446x slower
Significant (t=-9.542369)Stddev: 0.00141 -> 0.00291: 2.0585x larger (N = 50)

Running 'template_render_simple' benchmark ...
Min: 0.000113 -> 0.000080: 1.4119x faster
Avg: 0.000727 -> 0.000643: 1.1295x faster
Not significant
Stddev: 0.00410 -> 0.00366: 1.1216x smaller (N = 50)

Not sure whether the benchmarks test parsing or rendering, so feedback may be welcome.

comment:10 by Anssi Kääriäinen, 11 years ago

The test results seem somewhat random. Maybe there is some concurrent task eating CPU randomly? Or maybe CPU throttling is causing problems, the timing of the benchmarks surprisingly change if the CPU runs at basically random speed...

The results show that there isn't likely any big change.

Try to see if you can stabilise the results. If the results wont stabilise, I will try to benchmark this patch later on.

comment:11 by jonathanslenders, 11 years ago

After using "djangobench -t 200":

Running 'l10n_render' benchmark ...
Min: 0.004818 -> 0.004467: 1.0786x faster
Avg: 0.005679 -> 0.005978: 1.0527x slower
Significant (t=-2.910514)
Stddev: 0.00103 -> 0.00103: 1.0052x smaller (N = 200)

Running 'template_compilation' benchmark ...
Min: 0.000048 -> 0.000130: 2.7114x slower
Avg: 0.000560 -> 0.000572: 1.0203x slower
Not significant
Stddev: 0.00149 -> 0.00158: 1.0631x larger (N = 200)

Running 'template_render' benchmark ...
Min: 0.007676 -> 0.009130: 1.1894x slower
Avg: 0.010612 -> 0.011010: 1.0375x slower
Not significant
Stddev: 0.00384 -> 0.00181: 2.1225x smaller (N = 200)

Running 'template_render_simple' benchmark ...
Min: 0.000052 -> 0.000079: 1.5229x slower
Avg: 0.000346 -> 0.000179: 1.9358x faster
Not significant
Stddev: 0.00137 -> 0.00134: 1.0233x smaller (N = 200)

Not perfectly stable, but better. There was not really any other load on the machine.
I guess there's no notable difference in performance, but feel free to run the benchmarks again.

comment:12 by Anssi Kääriäinen, 11 years ago

I seem to get ~1.13x slower runtime for template_render benchmark, and I get this number consistently. But luckily this seems to be overhead from template parsing, not from rendering the template. This can be seen by changing the benchmark to render the same template 10 times, and changing the settings of the benchmark to use cached template loader.

So, I think performance is good enough.

comment:13 by Berker Peksag, 11 years ago

Cc: berker.peksag@… added
Note: See TracTickets for help on using tickets.
Back to Top