[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.

Ok. My reasoning was that extensions should hook in at as high a
level as possible (to ease maintenance etc.), and here the price
did not seem too high to me. But it's fine with me.
 
>> 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) ?

context.py would do: fctx._filerev is None ;-)

>> +            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?

I'm trying to find a test case -- perhaps you could give me a
command which reliably calls filectx.cmp, but not on the working
directory --, but in theory it should work, because keywords are
only expanded in the working directory.

> At the very least, if possible, I'd like to avoid repetitions.

I was aware of the repetition, but again I wanted to avoid to
expose too many internals in an extension. I wanted to act well
behaved and obey policies ;-) But going for speed is completely
fine with me personally.

> Inlining the code would make me feel safer. Would you mind?

At least we could pack it all in one condition.

I'm pressed for time. But if you could nudge me into the
direction of a test command that would be nice. I might try out
some other ways of doing this with the means of hgext.keyword,
perhaps on the (kw)filelog level. Just probably not today.

c
-- 
theatre - books - texts - movies
Black Trash Productions at home: http://www.blacktrash.org
Black Trash Productions on Facebook:
http://www.facebook.com/blacktrashproductions