NodeList.render cleanup

[(removed)]

Tucked away in NodeList.render is a fun little isinstance check, basically

if isinstance(node, Node):
  bits.append(self.render_node(node, context))
else:
  bits.append(node)

With the render method being a hook method for derivative classes to intercept exceptions, and add some debugging info in; two limiting points to scaling there
1) Because ForNode uses it to store strings, the isinstance is required- no other code uses it for this functionality, making me think ForNode shouldn't be forcing NodeList to do something odd, should do the job itself
2) Because DebugNodeList overrides render_node, instead of render, an extra func call is involved for every node; again, single usage slows down the common usage.

So... question is, why should ForNode use it? Have yet to find any comments/explanation code wise, only thing I can *guess* is that someone was trying to extend DebugNodeList.render_node behaviour down through a for; as said, this hurts the common case.

Realize the first reaction is likely going to be another "micro-optimization is evil" comment; thing is, while it's not a huge gain like say removing dispatch.send invocations from Models.*, it's a constant waste occuring in any NodeList.render (which are common enough). Usual usage scenario I'm abusing for testing pegs it as a constant 1.4s hit against trunk 64s (26s if using my misc patches sitting in tickets); might not seem like much, but it's yet another spot where it's spinning it's wheels without reason.

So... why is ForNode abusing NodeList here? Oversight? Worth preserving?

[(removed)]

NodeList/ForNode cleanup; ForNode uses a list directly, remove render_node from NodeList/DebugNodeList, instead moving the override into DebugNodeList and simplifying NodeList.render

[Malcolm Tredinnick]

One extra use-case to consider here is what other template tags might be using this. I don't know the answer to that, but changing the API here does have an effect on third-party template tags, so we have to weigh that up. For that reason, it's a backwards-incompatible change.

I'm +1 on this. The extra interface provided by NodeList (only get_nodes_by_type()) is easy to replicate if some other tags are using NodeList instead of a normal list for that purpose.

Will leave it for a little while to give Adrian or Jacob a chance to chime in, but it's worth doing, I think.

(Adding "performance" keyword as per Jacob's suggestion.)

[Malcolm Tredinnick]

We'll take #4565 and this patch is incorporated into that change.