Opened 7 years ago

Closed 4 years ago

#6586 closed Cleanup/optimization (wontfix)

extends tag should compile static parents

Reported by: fredd4@… Owned by: nobody
Component: Template system Version: master
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@… 7 years ago.
patch for extends file
extend.2.patch (1.4 KB) - added by fredd4@… 7 years ago.
Previous patch has a bug. Here is a good one.
extend.3.patch (1.4 KB) - added by fredd4 7 years ago.
Caching on render works better (more correctly)
extend_test.py (3.9 KB) - added by fredd4 7 years ago.
Performance extends test

Download all attachments as: .zip

Change History (19)

Changed 7 years ago by fredd4@…

patch for extends file

Changed 7 years ago by fredd4@…

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

Changed 7 years ago by fredd4

Caching on render works better (more correctly)

comment:1 Changed 7 years ago by forgems@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

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 Changed 7 years ago by anonymous

  • Resolution set to invalid
  • Status changed from new to closed

comment:3 Changed 7 years ago by fredd4

  • Resolution invalid deleted
  • Status changed from closed to reopened

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.

Changed 7 years ago by fredd4

Performance extends test

comment:4 Changed 7 years ago by forgems

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 Changed 7 years ago by fredd4

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

comment:6 Changed 7 years ago by forgems

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

comment:7 Changed 7 years ago by forgems

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 Changed 7 years ago by fredd4

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 Changed 7 years ago by forgems

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 Changed 7 years ago by SmileyChris

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

comment:11 Changed 7 years ago by fredd4

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 Changed 7 years ago by ericholscher

  • Triage Stage changed from Unreviewed to Design decision needed

comment:13 Changed 4 years ago by julien

  • Type set to Cleanup/optimization

comment:14 Changed 4 years ago by julien

  • Severity set to Normal

comment:15 Changed 4 years ago by carljm

  • Easy pickings unset
  • Resolution set to wontfix
  • Status changed from reopened to closed
  • 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