[PATCH 6 of 8] dirstate: create class for status lists

[Mads Kiilerich]

On 10/02/2014 04:26 PM, Martin von Zweigbergk wrote:
> On Thu, Oct 2, 2014 at 5:46 AM, Mads Kiilerich <mads at kiilerich.com> wrote:
>> On 10/02/2014 07:00 AM, Martin von Zweigbergk wrote:
>>> Thanks for taking a look! Answers below to some of your questions.
>>> I'll leave the others for later, or they might just get address in v2
>>> of the series.
>>>
>>> On Wed, Oct 1, 2014 at 6:40 PM, Mads Kiilerich <mads at kiilerich.com> wrote:
>>>> A general comment: A "." lookup is relatively expensive. It seems like
>>>> you
>>>> introduce a lot of them. We should take care to not introduce more of
>>>> them
>>>> in tight loops (that might already be the case - I did not check and not
>>>> see
>>>> any statement on whether that was the case).
>>> Hmm, I don't think I even considered that (I've never used python before).
>>>
>>> I tried running the test suite:
>>>
>>> Before:
>>> real    3m22.026s
>>> user    25m31.715s
>>> sys 5m45.684s
>>>
>>> After:
>>> real    3m24.136s
>>> user    25m29.724s
>>> sys     5m46.637s
>>>
>>> I also tried this:
>>>
>>> $ python -mtimeit -s'a=(1,2,3,4,5,6)' 'a[3]'
>>> 10000000 loops, best of 3: 0.0402 usec per loop
>>>
>>> $ python -mtimeit -s'class c(object):
>>>     def __init__(self,a,b,c,d,e,f):
>>>       self.a=a
>>>       self.b=b
>>>       self.c=c
>>>       self.d=d
>>>       self.e=e
>>>       self.f=f
>>> a=c(1,2,3,4,5,6)' 'a.d'
>>> 10000000 loops, best of 3: 0.0458 usec per loop
>>
>> Yes, these two are pretty much the same. But:
>>
>> $ python -mtimeit -s'a=(1,2,3,4,5,6)' -s 'b=a[0]' 'x=b'
>> 100000000 loops, best of 3: 0.0114 usec per loop
>>
>> $ python -mtimeit -s'class c(object):
>>    def __init__(self,a,b,c,d,e,f):
>>      self.a=a
>>      self.b=b
>>      self.c=c
>>      self.d=d
>>      self.e=e
>>      self.f=f
>> a=c(1,2,3,4,5,6)' 'x=a.d'
>> 10000000 loops, best of 3: 0.026 usec per loop
> That doesn't seem like a fair comparison to me. Assuming that '-s'
> means "this is setup, don't include in timing", the only thing your
> tuple test case does is to test assignment, right? So it's not
> comparing tuple lookup vs field name lookup.

Yes, I agree. The cost of a lookup is however relevant if we replace a 
modified.append(x) with status.modified.append(x). (The even better 
microoptimization would be to use modifiedappend=status.modified.append .)

/Mads