Patchwork [5,of,6,V5] manifest: get rid of manifest.readshallowfast

login
register
mail settings
Submitter Durham Goode
Date Nov. 3, 2016, 1:30 a.m.
Message ID <42cbc5a35d816cf9e5c0.1478136600@dev111.prn1.facebook.com>
Download mbox | patch
Permalink /patch/17307/
State Accepted
Headers show

Comments

Durham Goode - Nov. 3, 2016, 1:30 a.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1478131847 25200
#      Wed Nov 02 17:10:47 2016 -0700
# Branch stable
# Node ID 42cbc5a35d816cf9e5c0ab31791ab90df8004dab
# Parent  488f0af8cb92461a67b47bfe259e3378bb00769c
manifest: get rid of manifest.readshallowfast

This removes manifest.readshallowfast and converts it's one user to use
manifestlog instead.
via Mercurial-devel - Nov. 4, 2016, 10:43 p.m.
On Wed, Nov 2, 2016 at 6:30 PM, Durham Goode <durham@fb.com> wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1478131847 25200
> #      Wed Nov 02 17:10:47 2016 -0700
> # Branch stable
> # Node ID 42cbc5a35d816cf9e5c0ab31791ab90df8004dab
> # Parent  488f0af8cb92461a67b47bfe259e3378bb00769c
> manifest: get rid of manifest.readshallowfast
>
> This removes manifest.readshallowfast and converts it's one user to use
> manifestlog instead.
>
> diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
> --- a/mercurial/changegroup.py
> +++ b/mercurial/changegroup.py
> @@ -676,7 +676,8 @@ class cg1packer(object):
>      def generatemanifests(self, commonrevs, clrevorder, fastpathlinkrev, mfs,
>                            fnodes):
>          repo = self._repo
> -        dirlog = repo.manifest.dirlog
> +        mfl = repo.manifestlog
> +        dirlog = mfl._revlog.dirlog
>          tmfnodes = {'': mfs}
>
>          # Callback for the manifest, used to collect linkrevs for filelog
> @@ -704,7 +705,7 @@ class cg1packer(object):
>                  treemanifests to send.
>                  """
>                  clnode = tmfnodes[dir][x]
> -                mdata = dirlog(dir).readshallowfast(x)
> +                mdata = mfl.get(dir, x).readfast(shallow=True)
>                  for p, n, fl in mdata.iterentries():
>                      if fl == 't': # subdirectory manifest
>                          subdir = dir + p + '/'
> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
> --- a/mercurial/manifest.py
> +++ b/mercurial/manifest.py
> @@ -1351,6 +1351,12 @@ class manifestctx(object):
>          return self._data
>
>      def readfast(self, shallow=False):
> +        '''Calls either readdelta or read, based on which would be less work.
> +        readdelta is called if the delta is against the p1, and therefore can be
> +        read quickly.
> +
> +        If `shallow` is True, nothing changes since this is a flat manifest.
> +        '''
>          rl = self._repo.manifestlog._revlog
>          r = rl.rev(self._node)
>          deltaparent = rl.deltaparent(r)
> @@ -1421,7 +1427,7 @@ class treemanifestctx(object):
>          return self._node
>
>      def readdelta(self, shallow=False):
> -        revlog = self._revlog
> +        revlog = self._revlog()

Looks like this should have been in the previous patch.

>          if shallow and revlog._treeondisk and not revlog._usemanifestv2:
>              r = revlog.rev(self._node)
>              d = mdiff.patchtext(revlog.revdiff(revlog.deltaparent(r), r))
> @@ -1440,6 +1446,13 @@ class treemanifestctx(object):
>              return md
>
>      def readfast(self, shallow=False):
> +        '''Calls either readdelta or read, based on which would be less work.
> +        readdelta is called if the delta is against the p1, and therefore can be
> +        read quickly.
> +
> +        If `shallow` is True, it only returns the entries from this manifest,
> +        and not any submanifests.
> +        '''
>          rl = self._revlog()
>          r = rl.rev(self._node)
>          deltaparent = rl.deltaparent(r)
> @@ -1522,15 +1535,6 @@ class manifest(manifestrevlog):
>          d = mdiff.patchtext(self.revdiff(self.deltaparent(r), r))
>          return manifestdict(d)
>
> -    def readshallowfast(self, node):
> -        '''like readfast(), but calls readshallowdelta() instead of readdelta()
> -        '''
> -        r = self.rev(node)
> -        deltaparent = self.deltaparent(r)
> -        if deltaparent != revlog.nullrev and deltaparent in self.parentrevs(r):
> -            return self.readshallowdelta(node)
> -        return self.readshallow(node)
> -
>      def read(self, node):
>          if node == revlog.nullid:
>              return self._newmanifest() # don't upset local cache
> @@ -1558,13 +1562,6 @@ class manifest(manifestrevlog):
>              self.fulltextcache[node] = arraytext
>          return m
>
> -    def readshallow(self, node):
> -        '''Reads the manifest in this directory. When using flat manifests,
> -        this manifest will generally have files in subdirectories in it. Does
> -        not cache the manifest as the callers generally do not read the same
> -        version twice.'''
> -        return manifestdict(self.revision(node))
> -
>      def find(self, node, f):
>          '''look up entry for a single file efficiently.
>          return (node, flags) pair if found, (None, None) if not.'''
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Durham Goode - Nov. 4, 2016, 11:06 p.m.
On 11/4/16 3:43 PM, Martin von Zweigbergk wrote:
> On Wed, Nov 2, 2016 at 6:30 PM, Durham Goode <durham@fb.com> wrote:
>> # HG changeset patch
>> # User Durham Goode <durham@fb.com>
>> # Date 1478131847 25200
>> #      Wed Nov 02 17:10:47 2016 -0700
>> # Branch stable
>> # Node ID 42cbc5a35d816cf9e5c0ab31791ab90df8004dab
>> # Parent  488f0af8cb92461a67b47bfe259e3378bb00769c
>> manifest: get rid of manifest.readshallowfast
>>
>> This removes manifest.readshallowfast and converts it's one user to use
>> manifestlog instead.
>>
>> diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
>> --- a/mercurial/changegroup.py
>> +++ b/mercurial/changegroup.py
>> @@ -676,7 +676,8 @@ class cg1packer(object):
>>       def generatemanifests(self, commonrevs, clrevorder, fastpathlinkrev, mfs,
>>                             fnodes):
>>           repo = self._repo
>> -        dirlog = repo.manifest.dirlog
>> +        mfl = repo.manifestlog
>> +        dirlog = mfl._revlog.dirlog
>>           tmfnodes = {'': mfs}
>>
>>           # Callback for the manifest, used to collect linkrevs for filelog
>> @@ -704,7 +705,7 @@ class cg1packer(object):
>>                   treemanifests to send.
>>                   """
>>                   clnode = tmfnodes[dir][x]
>> -                mdata = dirlog(dir).readshallowfast(x)
>> +                mdata = mfl.get(dir, x).readfast(shallow=True)
>>                   for p, n, fl in mdata.iterentries():
>>                       if fl == 't': # subdirectory manifest
>>                           subdir = dir + p + '/'
>> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
>> --- a/mercurial/manifest.py
>> +++ b/mercurial/manifest.py
>> @@ -1351,6 +1351,12 @@ class manifestctx(object):
>>           return self._data
>>
>>       def readfast(self, shallow=False):
>> +        '''Calls either readdelta or read, based on which would be less work.
>> +        readdelta is called if the delta is against the p1, and therefore can be
>> +        read quickly.
>> +
>> +        If `shallow` is True, nothing changes since this is a flat manifest.
>> +        '''
>>           rl = self._repo.manifestlog._revlog
>>           r = rl.rev(self._node)
>>           deltaparent = rl.deltaparent(r)
>> @@ -1421,7 +1427,7 @@ class treemanifestctx(object):
>>           return self._node
>>
>>       def readdelta(self, shallow=False):
>> -        revlog = self._revlog
>> +        revlog = self._revlog()
> Looks like this should have been in the previous patch.
Woops, yes.  I'm not sure how the tests pass with this :/
via Mercurial-devel - Nov. 4, 2016, 11:17 p.m.
On Fri, Nov 4, 2016 at 4:06 PM, Durham Goode <durham@fb.com> wrote:
>
>
> On 11/4/16 3:43 PM, Martin von Zweigbergk wrote:
>>
>> On Wed, Nov 2, 2016 at 6:30 PM, Durham Goode <durham@fb.com> wrote:
>>>
>>> # HG changeset patch
>>> # User Durham Goode <durham@fb.com>
>>> # Date 1478131847 25200
>>> #      Wed Nov 02 17:10:47 2016 -0700
>>> # Branch stable
>>> # Node ID 42cbc5a35d816cf9e5c0ab31791ab90df8004dab
>>> # Parent  488f0af8cb92461a67b47bfe259e3378bb00769c
>>> manifest: get rid of manifest.readshallowfast
>>>
>>> This removes manifest.readshallowfast and converts it's one user to use
>>> manifestlog instead.
>>>
>>> diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
>>> --- a/mercurial/changegroup.py
>>> +++ b/mercurial/changegroup.py
>>> @@ -676,7 +676,8 @@ class cg1packer(object):
>>>       def generatemanifests(self, commonrevs, clrevorder,
>>> fastpathlinkrev, mfs,
>>>                             fnodes):
>>>           repo = self._repo
>>> -        dirlog = repo.manifest.dirlog
>>> +        mfl = repo.manifestlog
>>> +        dirlog = mfl._revlog.dirlog
>>>           tmfnodes = {'': mfs}
>>>
>>>           # Callback for the manifest, used to collect linkrevs for
>>> filelog
>>> @@ -704,7 +705,7 @@ class cg1packer(object):
>>>                   treemanifests to send.
>>>                   """
>>>                   clnode = tmfnodes[dir][x]
>>> -                mdata = dirlog(dir).readshallowfast(x)
>>> +                mdata = mfl.get(dir, x).readfast(shallow=True)
>>>                   for p, n, fl in mdata.iterentries():
>>>                       if fl == 't': # subdirectory manifest
>>>                           subdir = dir + p + '/'
>>> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
>>> --- a/mercurial/manifest.py
>>> +++ b/mercurial/manifest.py
>>> @@ -1351,6 +1351,12 @@ class manifestctx(object):
>>>           return self._data
>>>
>>>       def readfast(self, shallow=False):
>>> +        '''Calls either readdelta or read, based on which would be less
>>> work.
>>> +        readdelta is called if the delta is against the p1, and
>>> therefore can be
>>> +        read quickly.
>>> +
>>> +        If `shallow` is True, nothing changes since this is a flat
>>> manifest.
>>> +        '''
>>>           rl = self._repo.manifestlog._revlog
>>>           r = rl.rev(self._node)
>>>           deltaparent = rl.deltaparent(r)
>>> @@ -1421,7 +1427,7 @@ class treemanifestctx(object):
>>>           return self._node
>>>
>>>       def readdelta(self, shallow=False):
>>> -        revlog = self._revlog
>>> +        revlog = self._revlog()
>>
>> Looks like this should have been in the previous patch.
>
> Woops, yes.  I'm not sure how the tests pass with this :/

Pushed to committed with this fixed, as well as the other thing I
commented on (the unnecessary treeondisk in a condition). Thanks!

Patch

diff --git a/mercurial/changegroup.py b/mercurial/changegroup.py
--- a/mercurial/changegroup.py
+++ b/mercurial/changegroup.py
@@ -676,7 +676,8 @@  class cg1packer(object):
     def generatemanifests(self, commonrevs, clrevorder, fastpathlinkrev, mfs,
                           fnodes):
         repo = self._repo
-        dirlog = repo.manifest.dirlog
+        mfl = repo.manifestlog
+        dirlog = mfl._revlog.dirlog
         tmfnodes = {'': mfs}
 
         # Callback for the manifest, used to collect linkrevs for filelog
@@ -704,7 +705,7 @@  class cg1packer(object):
                 treemanifests to send.
                 """
                 clnode = tmfnodes[dir][x]
-                mdata = dirlog(dir).readshallowfast(x)
+                mdata = mfl.get(dir, x).readfast(shallow=True)
                 for p, n, fl in mdata.iterentries():
                     if fl == 't': # subdirectory manifest
                         subdir = dir + p + '/'
diff --git a/mercurial/manifest.py b/mercurial/manifest.py
--- a/mercurial/manifest.py
+++ b/mercurial/manifest.py
@@ -1351,6 +1351,12 @@  class manifestctx(object):
         return self._data
 
     def readfast(self, shallow=False):
+        '''Calls either readdelta or read, based on which would be less work.
+        readdelta is called if the delta is against the p1, and therefore can be
+        read quickly.
+
+        If `shallow` is True, nothing changes since this is a flat manifest.
+        '''
         rl = self._repo.manifestlog._revlog
         r = rl.rev(self._node)
         deltaparent = rl.deltaparent(r)
@@ -1421,7 +1427,7 @@  class treemanifestctx(object):
         return self._node
 
     def readdelta(self, shallow=False):
-        revlog = self._revlog
+        revlog = self._revlog()
         if shallow and revlog._treeondisk and not revlog._usemanifestv2:
             r = revlog.rev(self._node)
             d = mdiff.patchtext(revlog.revdiff(revlog.deltaparent(r), r))
@@ -1440,6 +1446,13 @@  class treemanifestctx(object):
             return md
 
     def readfast(self, shallow=False):
+        '''Calls either readdelta or read, based on which would be less work.
+        readdelta is called if the delta is against the p1, and therefore can be
+        read quickly.
+
+        If `shallow` is True, it only returns the entries from this manifest,
+        and not any submanifests.
+        '''
         rl = self._revlog()
         r = rl.rev(self._node)
         deltaparent = rl.deltaparent(r)
@@ -1522,15 +1535,6 @@  class manifest(manifestrevlog):
         d = mdiff.patchtext(self.revdiff(self.deltaparent(r), r))
         return manifestdict(d)
 
-    def readshallowfast(self, node):
-        '''like readfast(), but calls readshallowdelta() instead of readdelta()
-        '''
-        r = self.rev(node)
-        deltaparent = self.deltaparent(r)
-        if deltaparent != revlog.nullrev and deltaparent in self.parentrevs(r):
-            return self.readshallowdelta(node)
-        return self.readshallow(node)
-
     def read(self, node):
         if node == revlog.nullid:
             return self._newmanifest() # don't upset local cache
@@ -1558,13 +1562,6 @@  class manifest(manifestrevlog):
             self.fulltextcache[node] = arraytext
         return m
 
-    def readshallow(self, node):
-        '''Reads the manifest in this directory. When using flat manifests,
-        this manifest will generally have files in subdirectories in it. Does
-        not cache the manifest as the callers generally do not read the same
-        version twice.'''
-        return manifestdict(self.revision(node))
-
     def find(self, node, f):
         '''look up entry for a single file efficiently.
         return (node, flags) pair if found, (None, None) if not.'''