[PATCH] keyword: only use expensive fctx.cmp when needed (improved)

[Christian Ebert]

* Nicolas Dumazet on Thursday, October 14, 2010 at 11:09:40 +0200
> 2010/10/13 Christian Ebert <blacktrash at gmx.net>:
>> # HG changeset patch
>> # User Christian Ebert <blacktrash at gmx.net>
>> # Date 1286972860 -3600
>> # Node ID 8fd35de5f7ad821f008b26ef4f29a663b85f9fe2
>> # Parent  52971985be14f12930a9809d375b97e1ee77d21a
>> keyword: only use expensive fctx.cmp when needed
>> 
>> Restrict expensive cmp only when:
>> - comparing against working dir
>> - path is configured for keyword expansion
> 
> Thanks for the precisions. Not being aware of how keywords work, I
> trust this and thank you for the improvements.
> 
>> 
>> Prefer filelog(), filenode() methods over filectx' internal
>> _filelog, _filenode attributes.
> 
> Why? Since this is precisely a patch of filectx behavior, using
> internal attributes looks okay to me.
> The whole reasoning is to try to be as fast as we can: the less calls,
> the better we'll fare.
> 
>> 
>> diff --git a/hgext/keyword.py b/hgext/keyword.py
>> --- a/hgext/keyword.py
>> +++ b/hgext/keyword.py
>> @@ -594,9 +594,12 @@
>>     def kwfilectx_cmp(orig, self, fctx):
>>         # keyword affects data size, comparing wdir and filelog size does
>>         # not make sense
>> -        return self._filelog.cmp(self._filenode, fctx.data())
>> +        if (fctx.filerev() is None and
> 
> isinstance(fctx, workingfilecontext) ?
> 
>> +            kwt.match(fctx.path()) and not 'l' in fctx.flags()):
>> +            return self.filelog().cmp(self.filenode(), fctx.data())
> 
>> +        return orig(self, fctx)
> Does it work?
> The code for orig() should be something like:
>       if (isinstance(fctx, workingfilectx) and self._repo._encodefilterpats or
>           self.size() == fctx.size()):
>           return self._filelog.cmp(self._filenode, fctx.data())
>       return True
> I think that size() calls when keyword extension is activated do not
> really give reliable results. Or do they?
> 
> At the very least, if possible, I'd like to avoid repetitions.
> Inlining the code would make me feel safer. Would you mind?

I've just submitted a version that does this -- if I understood
you correctly. Note that the current state is buggy because you'd
also have to test for _encodefilterpats within keyword.

With this version (using orig(...)) the only call that is
repeated is the check for workingfilectx. The new version copies
the code from context and adds to the condition. Hm.

c
-- 
  Was heißt hier Dogma, ich bin Underdogma!
[ What the hell do you mean dogma, I am underdogma. ]
free movies       --->>> http://www.blacktrash.org/underdogma
http://itunes.apple.com/podcast/underdogma-movies/id363423596