Opened 11 years ago
Closed 9 years ago
#22086 closed Cleanup/optimization (wontfix)
Building of template nodelist inside cache blocks
Reported by: | Henrik Ossipoff Hansen | Owned by: | nobody |
---|---|---|---|
Component: | Template system | Version: | 1.6 |
Severity: | Normal | Keywords: | template, cache |
Cc: | Triage Stage: | Accepted | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
Based off of a rather large application in Django, our templates have grown quite complex. As such, when a template is compiled, it contains a huge bunch of nodes in the template's nodelist. Without knowing too much about the actual internals, my it is my understanding that anything included within a {% cache %}{% endcache %}
tag will still have its template nodes built, just not rendered - this is handled by the caching mechanism instead.
While this obviously is alright in the most cases, we have several places in our codebase where this is not enough; yes, the caching makes the rendering/implementation of the individual nodes not happen, which obviously is the whole point of the caching, but just representing the individual nodes in the node tree, inside the cache block, takes up a lot of time. In one of our application parts, we've timed this ti between 100ms and 200ms (based off inserting just the actual HTML stored in the cache instead of the Django template logic).
Now, there would obviously be an overhead in making a cache check before building the nodelist, and in most cases, I assume it's not needed - but I believe an option to enable this behaviour is in order. I would, at least, like to start a discussion about it - I'm sure we're not the only ones out there who would benefit from such an option.
Attachments (2)
Change History (6)
comment:1 by , 11 years ago
Triage Stage: | Unreviewed → Accepted |
---|
by , 11 years ago
Attachment: | menu_dk.html added |
---|
by , 11 years ago
Attachment: | games.html added |
---|
comment:2 by , 11 years ago
You are absolutely right - at some point the cached node will need to be rendered, at which point the nodelist inside the cached block will become irrelevant - it will just load whatever is cached instead and ignore the underlying tree.
But the tree still exists under the cached block until that point, and that seems to be the performance bottleneck we're experiencing, the fact that the tree needs to be built.
I've attached two of our template files to perhaps give a better understanding. menu_dk.html
is being included from our base.html
template, which forms the basis of all of our other templates. If you look at that template, it's mostly just a cache block with other files included within that cache block. So far so good.
Then comes one of the included files, games.html
. If you look through a bit of that, you see that nearly everything in it is custom template tags, namely url-translate
and facet-translate
. We know this is an ugly way to do it, but it's needed for our application at the moment - each of these tags generates a node that will, in their render()
, do a look-up in our product catalogue to find the correct translation for whatever language we're showing. No code of value is in the nodes' __init__
.
From my findings, it is the sheer number of nodes in the sub-tree of nodes inside the cache block that makes the template slow, even when the whole block is in fact cached. Sure, the nodes aren't rendered, but they still exist in the tree, causing up to 100ms to 200ms of extra time for our application.
In our particular case, it seems it would be an optimization if we could somehow tell the cache block to only built its inside nodelist if there didn't exist a valid cache for the block. In most cases, this check would likely take up more time than just building the tree, while in extreme cases of alot of nodes inside the cache block, it would be an optimization.
Hope this made it more clear, otherwise I'll be happy to provide more code if need be.
comment:3 by , 9 years ago
I did some investigating and I don't think this is possible. CacheNode
can only determine if something is in the cache at render time (it needs the context to look up vary_on
). The "inner" node list is built at parse time by do_cache
.
Even if moving the cache lookup to do_cache
was possible I would advise against it. It could cause (performance) issues by making the parser do cache lookup, way ahead of rendering.
In your specific case I would look into what your custom tags __init__
's are doing and, if possible, to defer as much as possible to render
.
I'm not completely sure I see how the optimisation you describe will help here -- at some point, the cached node will need to be rendered, which will mean the node tree will need to be parsed and stored.
However, you've clearly found a performance bottleneck, which deserves to be investigated.
If you could provide details of the template(s) you're using to manifest this problem, it would be very helpful for whoever undertakes this task.