Patchwork [2,of,4] manifest: adds manifestctx.readfast

login
register
mail settings
Submitter Durham Goode
Date Sept. 13, 2016, 11:30 p.m.
Message ID <f5b0e6f84938d661ee32.1473809445@dev111.prn1.facebook.com>
Download mbox | patch
Permalink /patch/16611/
State Accepted
Headers show

Comments

Durham Goode - Sept. 13, 2016, 11:30 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1473809190 25200
#      Tue Sep 13 16:26:30 2016 -0700
# Node ID f5b0e6f84938d661ee32b2303cb17e30a3fbb9df
# Parent  f63c46ee248044f73f51a5edc9c7be419df8ff39
manifest: adds manifestctx.readfast

This adds a copy of manifest.readfast to manifestctx.readfast and adds a
consumer of it. It currently looks like duplicate code, but a future patch
causes these functions to diverge as tree concepts are added to the tree
version.
via Mercurial-devel - Sept. 14, 2016, 2:43 a.m.
On Tue, Sep 13, 2016, 16:31 Durham Goode <durham@fb.com> wrote:

> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1473809190 25200
> #      Tue Sep 13 16:26:30 2016 -0700
> # Node ID f5b0e6f84938d661ee32b2303cb17e30a3fbb9df
> # Parent  f63c46ee248044f73f51a5edc9c7be419df8ff39
> manifest: adds manifestctx.readfast
>
> This adds a copy of manifest.readfast to manifestctx.readfast and adds a
> consumer of it. It currently looks like duplicate code, but a future patch
> causes these functions to diverge as tree concepts are added to the tree
> version.
>
> diff --git a/mercurial/context.py b/mercurial/context.py
> --- a/mercurial/context.py
> +++ b/mercurial/context.py
> @@ -824,7 +824,7 @@ class basefilectx(object):
>          """
>          repo = self._repo
>          cl = repo.unfiltered().changelog
> -        ma = repo.manifest
> +        mfl = repo.manifestlog
>          # fetch the linkrev
>          fr = filelog.rev(fnode)
>          lkr = filelog.linkrev(fr)
> @@ -849,7 +849,7 @@ class basefilectx(object):
>                  if path in ac[3]: # checking the 'files' field.
>                      # The file has been touched, check if the content is
>                      # similar to the one we search for.
> -                    if fnode == ma.readfast(ac[0]).get(path):
> +                    if fnode == mfl[ac[0]].readfast().get(path):
>                          return a
>              # In theory, we should never get out of that loop without a
> result.
>              # But if manifest uses a buggy file revision (not children of
> the
> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
> --- a/mercurial/manifest.py
> +++ b/mercurial/manifest.py
> @@ -993,6 +993,14 @@ class manifestctx(object):
>                  self._data = manifestdict(text)
>          return self._data
>
> +    def readfast(self):
> +        rl = self._revlog
> +        r = rl.rev(self._node)
> +        deltaparent = rl.deltaparent(r)
> +        if deltaparent != revlog.nullrev and deltaparent in
> rl.parentrevs(r):
> +            return self.readdelta()
> +        return self.read()
> +
>      def readdelta(self):
>          revlog = self._revlog
>          if revlog._usemanifestv2:
> @@ -1066,6 +1074,14 @@ class treemanifestctx(object):
>                      md.setflag(f, fl1)
>          return md
>
> +    def readfast(self):
> +        rl = self._revlog
> +        r = rl.rev(self._node)
> +        deltaparent = rl.deltaparent(r)
> +        if deltaparent != revlog.nullrev and deltaparent in
> rl.parentrevs(r):
> +            return self.readdelta()
> +        return self.read()
> +
>  class manifest(manifestrevlog):
>      def __init__(self, opener, dir='', dirlogcache=None):
>          '''The 'dir' and 'dirlogcache' arguments are for internal use by
> @@ -1149,20 +1165,6 @@ class manifest(manifestrevlog):
>          d = mdiff.patchtext(self.revdiff(self.deltaparent(r), r))
>          return manifestdict(d)
>
> -    def readfast(self, node):
> -        '''use the faster of readdelta or read
> -
> -        This will return a manifest which is either only the files
> -        added/modified relative to p1, or all files in the
> -        manifest. Which one is returned depends on the codepath used
> -        to retrieve the data.
> -        '''
> -        r = self.rev(node)
> -        deltaparent = self.deltaparent(r)
> -        if deltaparent != revlog.nullrev and deltaparent in
> self.parentrevs(r):
> -            return self.readdelta(node)
> -        return self.read(node)
> -
>

While reading the previous patch, I was wondering why you didn't delete
readdelta from manifest.manifest. Now I wonder even more.

     def readshallowfast(self, node):
>          '''like readfast(), but calls readshallowdelta() instead of
> readdelta()
>          '''
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
via Mercurial-devel - Sept. 14, 2016, 5:27 a.m.
On Tue, Sep 13, 2016 at 7:43 PM, Martin von Zweigbergk
<martinvonz@google.com> wrote:
>
>
> On Tue, Sep 13, 2016, 16:31 Durham Goode <durham@fb.com> wrote:
>>
>> # HG changeset patch
>> # User Durham Goode <durham@fb.com>
>> # Date 1473809190 25200
>> #      Tue Sep 13 16:26:30 2016 -0700
>> # Node ID f5b0e6f84938d661ee32b2303cb17e30a3fbb9df
>> # Parent  f63c46ee248044f73f51a5edc9c7be419df8ff39
>> manifest: adds manifestctx.readfast
>>
>> This adds a copy of manifest.readfast to manifestctx.readfast and adds a
>> consumer of it. It currently looks like duplicate code, but a future patch
>> causes these functions to diverge as tree concepts are added to the tree
>> version.
>>
>> diff --git a/mercurial/context.py b/mercurial/context.py
>> --- a/mercurial/context.py
>> +++ b/mercurial/context.py
>> @@ -824,7 +824,7 @@ class basefilectx(object):
>>          """
>>          repo = self._repo
>>          cl = repo.unfiltered().changelog
>> -        ma = repo.manifest
>> +        mfl = repo.manifestlog
>>          # fetch the linkrev
>>          fr = filelog.rev(fnode)
>>          lkr = filelog.linkrev(fr)
>> @@ -849,7 +849,7 @@ class basefilectx(object):
>>                  if path in ac[3]: # checking the 'files' field.
>>                      # The file has been touched, check if the content is
>>                      # similar to the one we search for.
>> -                    if fnode == ma.readfast(ac[0]).get(path):
>> +                    if fnode == mfl[ac[0]].readfast().get(path):
>>                          return a
>>              # In theory, we should never get out of that loop without a
>> result.
>>              # But if manifest uses a buggy file revision (not children of
>> the
>> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
>> --- a/mercurial/manifest.py
>> +++ b/mercurial/manifest.py
>> @@ -993,6 +993,14 @@ class manifestctx(object):
>>                  self._data = manifestdict(text)
>>          return self._data
>>
>> +    def readfast(self):
>> +        rl = self._revlog
>> +        r = rl.rev(self._node)
>> +        deltaparent = rl.deltaparent(r)
>> +        if deltaparent != revlog.nullrev and deltaparent in
>> rl.parentrevs(r):
>> +            return self.readdelta()
>> +        return self.read()
>> +
>>      def readdelta(self):
>>          revlog = self._revlog
>>          if revlog._usemanifestv2:
>> @@ -1066,6 +1074,14 @@ class treemanifestctx(object):
>>                      md.setflag(f, fl1)
>>          return md
>>
>> +    def readfast(self):
>> +        rl = self._revlog
>> +        r = rl.rev(self._node)
>> +        deltaparent = rl.deltaparent(r)
>> +        if deltaparent != revlog.nullrev and deltaparent in
>> rl.parentrevs(r):
>> +            return self.readdelta()
>> +        return self.read()
>> +
>>  class manifest(manifestrevlog):
>>      def __init__(self, opener, dir='', dirlogcache=None):
>>          '''The 'dir' and 'dirlogcache' arguments are for internal use by
>> @@ -1149,20 +1165,6 @@ class manifest(manifestrevlog):
>>          d = mdiff.patchtext(self.revdiff(self.deltaparent(r), r))
>>          return manifestdict(d)
>>
>> -    def readfast(self, node):
>> -        '''use the faster of readdelta or read
>> -
>> -        This will return a manifest which is either only the files
>> -        added/modified relative to p1, or all files in the
>> -        manifest. Which one is returned depends on the codepath used
>> -        to retrieve the data.
>> -        '''
>> -        r = self.rev(node)
>> -        deltaparent = self.deltaparent(r)
>> -        if deltaparent != revlog.nullrev and deltaparent in
>> self.parentrevs(r):
>> -            return self.readdelta(node)
>> -        return self.read(node)
>> -
>
>
> While reading the previous patch, I was wondering why you didn't delete
> readdelta from manifest.manifest. Now I wonder even more.

I see now, manifest.readdelta() is still used from other methods on
manifest, but manifest.readfast() is not, so you can remove it.


>
>>      def readshallowfast(self, node):
>>          '''like readfast(), but calls readshallowdelta() instead of
>> readdelta()
>>          '''
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Durham Goode - Sept. 14, 2016, 4:28 p.m.
On 9/13/16 10:27 PM, Martin von Zweigbergk wrote:
> On Tue, Sep 13, 2016 at 7:43 PM, Martin von Zweigbergk
> <martinvonz@google.com> wrote:
>>
>> On Tue, Sep 13, 2016, 16:31 Durham Goode <durham@fb.com> wrote:
>>> # HG changeset patch
>>> # User Durham Goode <durham@fb.com>
>>> # Date 1473809190 25200
>>> #      Tue Sep 13 16:26:30 2016 -0700
>>> # Node ID f5b0e6f84938d661ee32b2303cb17e30a3fbb9df
>>> # Parent  f63c46ee248044f73f51a5edc9c7be419df8ff39
>>> manifest: adds manifestctx.readfast
>>>
>>> This adds a copy of manifest.readfast to manifestctx.readfast and adds a
>>> consumer of it. It currently looks like duplicate code, but a future patch
>>> causes these functions to diverge as tree concepts are added to the tree
>>> version.
>>>
>>> diff --git a/mercurial/context.py b/mercurial/context.py
>>> --- a/mercurial/context.py
>>> +++ b/mercurial/context.py
>>> @@ -824,7 +824,7 @@ class basefilectx(object):
>>>           """
>>>           repo = self._repo
>>>           cl = repo.unfiltered().changelog
>>> -        ma = repo.manifest
>>> +        mfl = repo.manifestlog
>>>           # fetch the linkrev
>>>           fr = filelog.rev(fnode)
>>>           lkr = filelog.linkrev(fr)
>>> @@ -849,7 +849,7 @@ class basefilectx(object):
>>>                   if path in ac[3]: # checking the 'files' field.
>>>                       # The file has been touched, check if the content is
>>>                       # similar to the one we search for.
>>> -                    if fnode == ma.readfast(ac[0]).get(path):
>>> +                    if fnode == mfl[ac[0]].readfast().get(path):
>>>                           return a
>>>               # In theory, we should never get out of that loop without a
>>> result.
>>>               # But if manifest uses a buggy file revision (not children of
>>> the
>>> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
>>> --- a/mercurial/manifest.py
>>> +++ b/mercurial/manifest.py
>>> @@ -993,6 +993,14 @@ class manifestctx(object):
>>>                   self._data = manifestdict(text)
>>>           return self._data
>>>
>>> +    def readfast(self):
>>> +        rl = self._revlog
>>> +        r = rl.rev(self._node)
>>> +        deltaparent = rl.deltaparent(r)
>>> +        if deltaparent != revlog.nullrev and deltaparent in
>>> rl.parentrevs(r):
>>> +            return self.readdelta()
>>> +        return self.read()
>>> +
>>>       def readdelta(self):
>>>           revlog = self._revlog
>>>           if revlog._usemanifestv2:
>>> @@ -1066,6 +1074,14 @@ class treemanifestctx(object):
>>>                       md.setflag(f, fl1)
>>>           return md
>>>
>>> +    def readfast(self):
>>> +        rl = self._revlog
>>> +        r = rl.rev(self._node)
>>> +        deltaparent = rl.deltaparent(r)
>>> +        if deltaparent != revlog.nullrev and deltaparent in
>>> rl.parentrevs(r):
>>> +            return self.readdelta()
>>> +        return self.read()
>>> +
>>>   class manifest(manifestrevlog):
>>>       def __init__(self, opener, dir='', dirlogcache=None):
>>>           '''The 'dir' and 'dirlogcache' arguments are for internal use by
>>> @@ -1149,20 +1165,6 @@ class manifest(manifestrevlog):
>>>           d = mdiff.patchtext(self.revdiff(self.deltaparent(r), r))
>>>           return manifestdict(d)
>>>
>>> -    def readfast(self, node):
>>> -        '''use the faster of readdelta or read
>>> -
>>> -        This will return a manifest which is either only the files
>>> -        added/modified relative to p1, or all files in the
>>> -        manifest. Which one is returned depends on the codepath used
>>> -        to retrieve the data.
>>> -        '''
>>> -        r = self.rev(node)
>>> -        deltaparent = self.deltaparent(r)
>>> -        if deltaparent != revlog.nullrev and deltaparent in
>>> self.parentrevs(r):
>>> -            return self.readdelta(node)
>>> -        return self.read(node)
>>> -
>>
>> While reading the previous patch, I was wondering why you didn't delete
>> readdelta from manifest.manifest. Now I wonder even more.
> I see now, manifest.readdelta() is still used from other methods on
> manifest, but manifest.readfast() is not, so you can remove it.
Correct.  readdelta is used by the shallow functions, so it will be 
deleted in the same patch that removes the last use of readshallowdelta.

Patch

diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -824,7 +824,7 @@  class basefilectx(object):
         """
         repo = self._repo
         cl = repo.unfiltered().changelog
-        ma = repo.manifest
+        mfl = repo.manifestlog
         # fetch the linkrev
         fr = filelog.rev(fnode)
         lkr = filelog.linkrev(fr)
@@ -849,7 +849,7 @@  class basefilectx(object):
                 if path in ac[3]: # checking the 'files' field.
                     # The file has been touched, check if the content is
                     # similar to the one we search for.
-                    if fnode == ma.readfast(ac[0]).get(path):
+                    if fnode == mfl[ac[0]].readfast().get(path):
                         return a
             # In theory, we should never get out of that loop without a result.
             # But if manifest uses a buggy file revision (not children of the
diff --git a/mercurial/manifest.py b/mercurial/manifest.py
--- a/mercurial/manifest.py
+++ b/mercurial/manifest.py
@@ -993,6 +993,14 @@  class manifestctx(object):
                 self._data = manifestdict(text)
         return self._data
 
+    def readfast(self):
+        rl = self._revlog
+        r = rl.rev(self._node)
+        deltaparent = rl.deltaparent(r)
+        if deltaparent != revlog.nullrev and deltaparent in rl.parentrevs(r):
+            return self.readdelta()
+        return self.read()
+
     def readdelta(self):
         revlog = self._revlog
         if revlog._usemanifestv2:
@@ -1066,6 +1074,14 @@  class treemanifestctx(object):
                     md.setflag(f, fl1)
         return md
 
+    def readfast(self):
+        rl = self._revlog
+        r = rl.rev(self._node)
+        deltaparent = rl.deltaparent(r)
+        if deltaparent != revlog.nullrev and deltaparent in rl.parentrevs(r):
+            return self.readdelta()
+        return self.read()
+
 class manifest(manifestrevlog):
     def __init__(self, opener, dir='', dirlogcache=None):
         '''The 'dir' and 'dirlogcache' arguments are for internal use by
@@ -1149,20 +1165,6 @@  class manifest(manifestrevlog):
         d = mdiff.patchtext(self.revdiff(self.deltaparent(r), r))
         return manifestdict(d)
 
-    def readfast(self, node):
-        '''use the faster of readdelta or read
-
-        This will return a manifest which is either only the files
-        added/modified relative to p1, or all files in the
-        manifest. Which one is returned depends on the codepath used
-        to retrieve the data.
-        '''
-        r = self.rev(node)
-        deltaparent = self.deltaparent(r)
-        if deltaparent != revlog.nullrev and deltaparent in self.parentrevs(r):
-            return self.readdelta(node)
-        return self.read(node)
-
     def readshallowfast(self, node):
         '''like readfast(), but calls readshallowdelta() instead of readdelta()
         '''