[PATCH] Small optimization inTemplate._flatten

[Christoph Zwerschke <cito@…>]

I noticed two small issues in the loop in Template._flatten:

1. Is it clear that substream is either a list or a string and not any other kind of sequence? If not, shouldn't be other sequences treated like lists?
2. In the case that substream is a (unicode) string, it is reassembled by joining all its characters (in ​this line).

The attached patch fixes these problems.

[Christoph Zwerschke <cito@…>]

Patch for genshi.template.base

[anonymous]

Replying to Christoph Zwerschke <cito@…>:

I noticed two small issues in the loop in Template._flatten:

1. Is it clear that substream is either a list or a string and not any other kind of sequence? If not, shouldn't be other sequences treated like lists?

Yeah, that's clear, as that's what MarkupTemplate._interpolate_attrs​ will create. The switch on type(x) is list instead of not isinstance(x, basestring) is a tiny optimization that can make a difference when you have many static attributes in a template.

2. In the case that substream is a (unicode) string, it is reassembled by joining all its characters (in ​this line).

Ugh, good catch!

[cmlenz]

anonymous was me

[cmlenz]

Fixed in [1033].

[Christoph Zwerschke <cito@…>]

Btw, since we're already at it, has Genshi now given up backward compatibility with Py 2.3? If yes, then using generator expressions instead of building lists may gain some efficiency. In this case, you could write

    value = u''.join(event[1]
        for event in self._flatten(value, ctxt, **vars)
        if event[0] is TEXT and event[1] is not None)
    if not value:
        continue

instead of

    values = []
    for event in self._flatten(value, ctxt, **vars):
        if event[0] is TEXT:
            values.append(event[1])
    value = [x for x in values if x is not None]
    if not value:
        continue
    value = u''.join(value)

I think the generator expression is even easier to read.

[cmlenz]

There's a subtle difference here: in the generator expression version, you'll completely drop attributes that evaluate to an empty string. The existing implementation only drops attributes that evaluate to None.

Also, in my experience generator expressions tend to be slower than the equivalent list comprehensions for small data sets.

But that code should be converted to a single list comprehension (instead of a loop plus a list comprehension). I've checked that in in [1034].

[Christoph Zwerschke <cito@…>]

You're right about the None/empty issue, this can be better checked with the list comprehension.

You're also right about the performance advantage of list comprehensions. I just did some quick testing with timeit. It turned out that even for larger data sets, list comprehensions were significantly faster than their corresponding generator expressions. That was not what I expected. The reason is probably that the implementation of list comprehensions is optimized and doesn't call eval_frame() as it happens for generator expression.

[Christoph Zwerschke <cito@…>]

Btw, I repeated these tests with different Python versions from 2.4 to 3.1, with the same result. And just for fun I tested this with Iron Python as well. Turned out that generator expressions are dramatically slower in Iron Python (really ridiculously slow). So yet another reason to favor list comprehensions. But I think that's an ugly flaw in Python (and even more so in Iron Python).