[PATCH 3 of 7] context: fix introrev to avoid computation as initially intended

[Martin von Zweigbergk]

On Mon, Oct 1, 2018 at 9:11 AM Boris FELD <boris.feld at octobus.net> wrote:

> On 10/09/2018 18:21, Martin von Zweigbergk via Mercurial-devel wrote:
>
>
>
> On Fri, Sep 7, 2018 at 8:09 AM Boris Feld <boris.feld at octobus.net> wrote:
>
>> # HG changeset patch
>> # User Boris Feld <boris.feld at octobus.net>
>> # Date 1536254177 14400
>> #      Thu Sep 06 13:16:17 2018 -0400
>> # Node ID a4c3eb6c1a36cbbf64fa8930b173154b2e77ef2b
>> # Parent  9a18509c522deeb62a7b244dcf4c7b79a8dc1132
>> # EXP-Topic copy-perf
>> # Available At https://bitbucket.org/octobus/mercurial-devel/
>> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r
>> a4c3eb6c1a36
>> context: fix introrev to avoid computation as initially intended
>>
>> diff --git a/mercurial/context.py b/mercurial/context.py
>> --- a/mercurial/context.py
>> +++ b/mercurial/context.py
>> @@ -829,6 +829,23 @@ class basefilectx(object):
>>              # result is crash somewhere else at to some point.
>>          return lkr
>>
>> +    def _lazyrev(self):
>>
>
> We usually try to separate refactoring (like extracting a method) from
> functional (or performance-related) changes. Could you do that with this
> patch or does it not make sense for some reason?
>
> In this case, the two changes are a bit too interleaved to be easily split
> in two. We can't do the special casing until we have the new method and the
> new method can't have any caller without changing the conditionals to
> include the special casing.
>

Maybe I missed something, but it looks like _lazyrev() returns either
"None" or "self.rev()", even at the end of the series. Did I get that
right? Will that change later? If not, it seems like you could instead
extract a method that calculates what's currently called "noctx". Even if
that's going to change, it might make it easier to understand this patch if
you split out a patch that made the structure here more similar to your
goal, something like:


@@ -837,9 +837,13 @@ class basefilectx(object):
         lkr = self.linkrev()
         attrs = vars(self)
         noctx = not (r'_changeid' in attrs or r'_changectx' in attrs)
-        if noctx or self.rev() == lkr:
+        if noctx:
             return self.linkrev()
-        return self._adjustlinkrev(self.rev(), inclusive=True)
+        else:
+            if self.rev() == lkr:
+                return self.linkrev()
+            else:
+                return self._adjustlinkrev(self.rev(), inclusive=True)


>
> _______________________________________________
> Mercurial-devel mailing listMercurial-devel at mercurial-scm.orghttps://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20181001/4310e565/attachment.html>