Opened 16 years ago

Closed 13 years ago

#6586 closed Cleanup/optimization (wontfix)

extends tag should compile static parents

Reported by: fredd4@… Owned by: nobody
Component: Template system Version: dev
Severity: Normal Keywords: extends
Cc: Triage Stage: Design decision needed
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Extends tag compile its parents always on render. This is slowly. It should do this on init, if it is possible.

Attachments (4)

extend.patch (1.1 KB ) - added by fredd4@… 16 years ago.
patch for extends file
extend.2.patch (1.4 KB ) - added by fredd4@… 16 years ago.
Previous patch has a bug. Here is a good one.
extend.3.patch (1.4 KB ) - added by fredd4 16 years ago.
Caching on render works better (more correctly)
extend_test.py (3.9 KB ) - added by fredd4 16 years ago.
Performance extends test

Download all attachments as: .zip

Change History (19)

by fredd4@…, 16 years ago

Attachment: extend.patch added

patch for extends file

by fredd4@…, 16 years ago

Attachment: extend.2.patch added

Previous patch has a bug. Here is a good one.

by fredd4, 16 years ago

Attachment: extend.3.patch added

Caching on render works better (more correctly)

comment:1 by forgems@…, 16 years ago

Render method for extends tag is called exactly once per template ( because only one extends tag can exist in template), so template processing is called exactly once. So your code will always call process_parent. No speed gain here.

comment:2 by anonymous, 16 years ago

Resolution: invalid
Status: newclosed

comment:3 by fredd4, 16 years ago

Resolution: invalid
Status: closedreopened

Not exactly. There is no speed gain if you use render_to_response function. But when you are using once init:

t = get_template(template_name)

and then many times render:

t.render(context_instance)

The extend tag read and parse html file every render call. It shouldn't. Extend should keep in memory parents nodes.

In my path, here you have:

if self.parent_name: 
   self.compiled_parent = compiled_parent

Let's look at my test. I render page from admin, firstly using standard extends tag, and then using my extends tag.

by fredd4, 16 years ago

Attachment: extend_test.py added

Performance extends test

comment:4 by forgems, 16 years ago

You can use this snippet http://www.djangosnippets.org/snippets/507/ to have globally enabled caching for templates. Imho it's not worth to change extends tag when you can simply monkey patch the loader system.

comment:5 by fredd4, 16 years ago

This snippet does not fix it. Try run my test.

comment:6 by forgems, 16 years ago

What if extends argument is a variable (changed on every request) from context. Your code will not work correctly.

comment:7 by forgems, 16 years ago

You can pass a compiled template object as a variable to context and use as an argument for extend. You will save time for template lookup and parsing.

comment:8 by fredd4, 16 years ago

My code will work correctly. I think, You don't check it.

If argument is variable, then it is save to variable named parent_name_expr, otherwise it is save to parent_name.
In my code, there is something like this:

if self.parent_name:
    self.compiled_parent = compiled_parent

So I check it.

comment:9 by forgems, 16 years ago

I have cheked it and it seem ok. The only problem is when you change the parent's template code. You have to restart the server, so maybe you should change:

if self.compiled_parent:
            compiled_parent = self.compiled_parent

to

if self.compiled_parent and not settings.DEBUG:
            compiled_parent = self.compiled_parent

comment:10 by Chris Beaven, 16 years ago

See also #6262 - a more broad way of caching templates.

comment:11 by fredd4, 16 years ago

To cache template I use my decorator: http://www.djangosnippets.org/snippets/596/
But it is like Your caching using Your snippet: http://www.djangosnippets.org/snippets/507/

There shouldn't be settings.DEBUG, because it is cached only if you cached main template.
If you use render_to_response without modification, it never happend.

In other words, my patch works like that: if you have some template in cache, the parents are in cache too, otherwise they aren't.

It's clear?

comment:12 by Eric Holscher, 16 years ago

Triage Stage: UnreviewedDesign decision needed

comment:13 by Julien Phalip, 13 years ago

Type: Cleanup/optimization

comment:14 by Julien Phalip, 13 years ago

Severity: Normal

comment:15 by Carl Meyer, 13 years ago

Easy pickings: unset
Resolution: wontfix
Status: reopenedclosed
UI/UX: unset

This is already solved in the general case by the caching template loader - works for all template loading and compiling, not just as a special case for extends tag. So there's no need to complicate the code with a special case for extends.

Note: See TracTickets for help on using tickets.
Back to Top