Open Stack

On 10 June 2015 at 21:30, Ihar Hrachyshka <ihrachys at redhat.com> wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
>
> On 06/10/2015 02:15 AM, Robert Collins wrote:
>> I'm very glad folk are working on Python3 ports.
>>
>> I'd like to call attention to one little wart in that process: I
>> get the feeling that folk are applying a massive regex to find
>> things like d.iteritems() and convert that to six.iteritems(d).
>>
>> I'd very much prefer that such a regex approach move things to
>> d.items(), which is much easier to read.
>>
>> Here's why. Firstly, very very very few of our dict iterations are
>> going to be performance sensitive in the way that iteritems()
>> matters. Secondly, no really - unless you're doing HUGE dicts, it
>> doesn't matter. Thirdly. Really, it doesn't.
>>
>
> Does it hurt though? ;)

Yes.

Its: harder to read. Its going to have to be removed eventually anyway
(when we stop supporting 2.7). Its marginally slower on 3.x (it has a
function and an iterator wrapping the actual thing). Its unidiomatic,
and we get lots of programmers that are new to Python; we should be
giving them as beautiful code as we can to help them learn.

>> At 1 million items the overhead is 54ms[1]. If we're doing inner
>> loops on million item dictionaries anywhere in OpenStack today, we
>> have a problem. We might want to in e.g. the scheduler... if it
>> held in-memory state on a million hypervisors at once, because I
>> don't really to to imagine it pulling a million rows from a DB on
>> every action. But then, we'd be looking at a whole 54ms. I think we
>> could survive, if we did that (which we don't).
>>
>> So - please, no six.iteritems().
>>
>
> The reason why in e.g. neutron we merged the patch using six.iteritems
> is that we don't want to go too deep into determining whether the
> original usage of iteritems() was justified.

Its not.

> The goal of the patch is
> to get python3 support, not to apply subjective style guidelines, so
> if someone wants to eliminate .iteritems(), he should create another
> patch just for that and struggle with reviewing it. While folks
> interested python3 can proceed with their work.
>
> We should not be afraid of multiple patches.

We shouldn't be indeed. All I'm asking is that we don't do poor
intermediate patches.

I've written code where performance tuning like that around iteritems
mattered. That code also needed to optimise tuple unpacking to avoid
performance hits and was aiming to manipulate million item data sets
from interpreter startup in subsecond times. It was some of the worst,
most impenetrable Python code I've ever seen, and while our code has
lots of issues, it neither has the same performance context that that
did, nor (thankfully) is it such impenetrable code.

-Rob

-- 
Robert Collins <rbtcollins at hp.com>
Distinguished Technologist
HP Converged Cloud