Django

Code

Ticket #9874 (closed: duplicate)

Opened 1 year ago

Last modified 11 months ago

Added template cache in template loader

Reported by: novoya Assigned to: novoya
Milestone: Component: Template system
Version: SVN Keywords: template cache
Cc: Triage Stage: Design decision needed
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

The following view module fails to make good reuse to the template For each new request, all inherited/included templates will be recreated and compiled 

tpl = loader.get_template('mytemplate.html')

def view_func1(request):
     '''
        Any inherited templates from tpl
        will created and recompiled again and again
     '''
     return HttpResponse(tpl.render(context))

Attached a very simple patch to solve the problem: 1.Simple template caching in the django.template.loader 2.Minimum change in django.template.loader_tags

Attachments

template_cache.diff (2.0 kB) - added by novoya on 12/17/08 08:22:29.
templace cache patch
template-cache.diff (6.9 kB) - added by groks on 03/22/09 19:13:40.
Fixes for cache key name, thread safety. Adds config, tests.

Change History

12/17/08 08:22:29 changed by novoya

  • attachment template_cache.diff added.

templace cache patch

12/17/08 08:36:19 changed by novoya

  • owner changed from nobody to novoya.
  • needs_better_patch changed.
  • status changed from new to assigned.
  • needs_tests changed.
  • needs_docs changed.

12/17/08 10:14:24 changed by novoya

  • status changed from assigned to closed.
  • summary changed from Template reuse is worthless(when inheretiance or inclusion involved) because parent templates get re-compiled every time to Added template cache in template loader.
  • resolution set to fixed.
  • needs_tests set to 1.
  • milestone set to post-1.0.

12/17/08 13:37:53 changed by ElliottM

I'm confused, when was this fixed?

12/17/08 14:36:18 changed by ubernostrum

  • status changed from closed to reopened.
  • resolution deleted.

(follow-up: ↓ 8 ) 12/17/08 14:37:07 changed by ubernostrum

  • stage changed from Unreviewed to Design decision needed.

It wasn't fixed. Somebody needs to learn how to read the documentation on the ticket process to find out what "fixed" means (hint: not "some random person uploaded a patch").

(follow-up: ↓ 9 ) 12/17/08 17:44:55 changed by mtredinnick

This is almost certainly going to cause rednering bugs. The Python objects representating templates are not just the compiled form, they also (intentionally) contain rendering state. It also assumes that the same name will load the same template in every thread, which isn't true (a dynamic template loader, for example). So there are larger considerations that don't seem to have been addressed here.

Right now, this is probably the wrong approach to solving the perceived problem, but want to think about it some more.

12/18/08 16:12:10 changed by lukeplant

It also is a hit in terms of memory usage -- every single template used in the system is going to get stored in memory, if I read the patch correctly. This could be a much worse penalty than any benefit it gives. I read somewhere that Adrian and co had originally intended templates to be stored in some compiled form, but they realised later that it simply wasn't worth the overhead.

(in reply to: ↑ 5 ) 12/19/08 03:48:06 changed by novoya

Replying to ubernostrum:

It wasn't fixed. Somebody needs to learn how to read the documentation on the ticket process to find out what "fixed" means (hint: not "some random person uploaded a patch").

Yes. Sorry guys. My mistake. I isn't fixed Anyway, I didn't tested too much, but from my tests, my SIMPLE fix boosts performance considerably (for each refresh- 0 template reloads + compiles instead of 5 or more in a typical web app) About memory usage concerns, it is very easy to limit the size of the cache in code.

(in reply to: ↑ 6 ) 12/19/08 04:02:36 changed by novoya

Replying to mtredinnick:

This is almost certainly going to cause rednering bugs. The Python objects representating templates are not just the compiled form, they also (intentionally) contain rendering state. It also assumes that the same name will load the same template in every thread, which isn't true (a dynamic template loader, for example). So there are larger considerations that don't seem to have been addressed here. Right now, this is probably the wrong approach to solving the perceived problem, but want to think about it some more.

How can template contain rendering state, if each time a template needed, it gets reloaded and recompiled ??

02/19/09 17:30:45 changed by simon29

Some benchmarking might be useful here. I could imagine the benefit being negligent on simple templates; but it could give a big plus for high traffic templates and/or large, complex templates (eg with lots of includes, multiple levels of inheritance, loading lots of tag libraries). Maybe it could be a setting, eg TEMPLATE_CACHE = 100, set zero by default. I have noticed before that after the database has been optimised and memcache used wherever possible, a unnecessarily high number of cycles are chewed up on template rendering.

02/25/09 13:51:44 changed by

  • milestone deleted.

Milestone post-1.0 deleted

03/22/09 19:13:40 changed by groks

  • attachment template-cache.diff added.

Fixes for cache key name, thread safety. Adds config, tests.

03/22/09 19:30:38 changed by groks

  • needs_tests deleted.

Y'all, here's a new patch which fixes a couple of the concerns raised above:

Add TEMPLATE_CACHE config setting (default off), update the docs. Don't cache templates when we're in TEMPLATE_DEBUG mode. Use auxiliary template load dirs as part of cache key name. Add some locking around the template cache. Add some tests.

Not only is template caching faster, but it opens the door for further tuning. For example, we could walk the compiled template and coalesce adjacent TextNode?'s.

What else needs to be done to get this accepted?

05/04/09 18:25:02 changed by ramiro

See also #9154, it contains a patch with a similar implementation and it seems to take in account problems found with the {{ block.super}} variable.

05/05/09 03:03:10 changed by mrts

  • status changed from reopened to closed.
  • resolution set to duplicate.

Duplicate of #9154.

Add/Change #9874 (Added template cache in template loader)




Change Properties
Action