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

[Nicolas Dumazet]

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?

>     extensions.wrapfunction(context.filectx, 'cmp', kwfilectx_cmp)
> -
>     extensions.wrapfunction(patch.patchfile, '__init__', kwpatchfile_init)
>     extensions.wrapfunction(patch, 'diff', kw_diff)
>     extensions.wrapfunction(cmdutil, 'copy', kw_copy)
>



-- 
Nicolas Dumazet — NicDumZ