[PATCH 1 of 2 STABLE] repoview: make propertycache.setcache compatible

[Pierre-Yves David]

On 10 sept. 2013, at 16:31, David Soria Parra wrote:

> 
>> diff --git a/mercurial/util.py b/mercurial/util.py
>> --- a/mercurial/util.py
>> +++ b/mercurial/util.py
>> @@ -277,11 +277,12 @@ class propertycache(object):
>>         result = self.func(obj)
>>         self.cachevalue(obj, result)
>>         return result
>> 
>>     def cachevalue(self, obj, value):
>> -        setattr(obj, self.name, value)
>> +        # __dict__ assigment required to bypass __setattr__ (eg: repoview)
>> +        obj.__dict__[self.name] = value
> 
> I am not sure if this is the right way to do it. It will work at the
> moment and has the desired effect as we bypass __setattr__ magic. But n
> case some extension will use propertycache + some __set__ magic (e.g.
> redirecting it) we will end up with the same problem again. I think a
> more future proof version would be to store the value in the
> propertycache itself and keep propertycache objects.

I disagree with the __set__ approach here.

Storing the value in the property itself will have a significant performance cost. Simple attribute lookup is much faster that class lookup + descriptor detection + function call. (50 usages in core, some of them performance critical)

Some codes in core and extensions relies on the current behavior and explicitly check in __dict__ (or vars) (about 20 usages in core)

We are patching the "cachevalue" method of propertycache. This function should be the one overwritten by extension altering the way the propertycache store its value. (no bundled extension break with the change)

Our propertycache is just yet another instance of a common python idioms and the __dict__ version is often used[1].

[1] http://www.toofishes.net/blog/python-cached-property-decorator/ 

-- 
Pierre-Yves David