Patchwork [2,of,5] manifest: make manifest derive from manifestrevlog

login
register
mail settings
Submitter Durham Goode
Date Aug. 9, 2016, 1:17 a.m.
Message ID <6bcb94d28e6fa3a94299.1470705431@dev8486.prn1.facebook.com>
Download mbox | patch
Permalink /patch/16222/
State Superseded
Headers show

Comments

Durham Goode - Aug. 9, 2016, 1:17 a.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1470696725 25200
#      Mon Aug 08 15:52:05 2016 -0700
# Node ID 6bcb94d28e6fa3a9429926fec8a42f3f8699b8f5
# Parent  f91cdd4315bbc92ad893c8084c0347c218399ce3
manifest: make manifest derive from manifestrevlog

As part of our refactoring to split the manifest concept from its storage, we
need to start moving the revlog specific parts of the manifest implementation to
a new class. This patch creates manifestrevlog and moves the fulltextcache onto
the base class.
via Mercurial-devel - Aug. 12, 2016, 4:06 a.m.
On Mon, Aug 8, 2016 at 6:17 PM, Durham Goode <durham@fb.com> wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1470696725 25200
> #      Mon Aug 08 15:52:05 2016 -0700
> # Node ID 6bcb94d28e6fa3a9429926fec8a42f3f8699b8f5
> # Parent  f91cdd4315bbc92ad893c8084c0347c218399ce3
> manifest: make manifest derive from manifestrevlog
>
> As part of our refactoring to split the manifest concept from its storage, we
> need to start moving the revlog specific parts of the manifest implementation to
> a new class. This patch creates manifestrevlog and moves the fulltextcache onto
> the base class.
>
> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
> --- a/mercurial/manifest.py
> +++ b/mercurial/manifest.py
> @@ -890,7 +890,30 @@ class treemanifest(object):
>                  subp1, subp2 = subp2, subp1
>              writesubtree(subm, subp1, subp2)
>
> -class manifest(revlog.revlog):
> +class manifestrevlog(revlog.revlog):
> +    '''A revlog that stores manifest texts. This is responsible for caching the
> +    full-text manifest contents.
> +    '''
> +    def __init__(self, opener, indexfile):
> +        super(manifestrevlog, self).__init__(opener, indexfile)
> +
> +        # During normal operations, we expect to deal with not more than four
> +        # revs at a time (such as during commit --amend). When rebasing large
> +        # stacks of commits, the number can go up, hence the config knob below.
> +        cachesize = 4
> +        opts = getattr(opener, 'options', None)
> +        if opts is not None:
> +            cachesize = opts.get('manifestcachesize', cachesize)
> +        self._fulltextcache = util.lrucachedict(cachesize)
> +
> +    @property
> +    def fulltextcache(self):
> +        return self._fulltextcache
> +
> +    def clearcaches(self):
> +        self._fulltextcache.clear()
> +
> +class manifest(manifestrevlog):
>      def __init__(self, opener, dir='', dirlogcache=None):
>          '''The 'dir' and 'dirlogcache' arguments are for internal use by
>          manifest.manifest only. External users should create a root manifest
> @@ -908,7 +931,6 @@ class manifest(revlog.revlog):
>              usetreemanifest = opts.get('treemanifest', usetreemanifest)
>              usemanifestv2 = opts.get('manifestv2', usemanifestv2)
>          self._mancache = util.lrucachedict(cachesize)
> -        self._fulltextcache = util.lrucachedict(cachesize)
>          self._treeinmem = usetreemanifest
>          self._treeondisk = usetreemanifest
>          self._usemanifestv2 = usemanifestv2
> @@ -918,7 +940,7 @@ class manifest(revlog.revlog):
>              if not dir.endswith('/'):
>                  dir = dir + '/'
>              indexfile = "meta/" + dir + "00manifest.i"
> -        revlog.revlog.__init__(self, opener, indexfile)
> +        super(manifest, self).__init__(opener, indexfile)
>          self._dir = dir
>          # The dirlogcache is kept on the root manifest log
>          if dir:
> @@ -1016,7 +1038,7 @@ class manifest(revlog.revlog):
>              m = self._newmanifest(text)
>              arraytext = array.array('c', text)
>          self._mancache[node] = m
> -        self._fulltextcache[node] = arraytext
> +        self.fulltextcache[node] = arraytext
>          return m
>
>      def readshallow(self, node):
> @@ -1036,7 +1058,7 @@ class manifest(revlog.revlog):
>              return None, None
>
>      def add(self, m, transaction, link, p1, p2, added, removed):
> -        if (p1 in self._fulltextcache and not self._treeinmem
> +        if (p1 in self.fulltextcache and not self._treeinmem
>              and not self._usemanifestv2):
>              # If our first parent is in the manifest cache, we can
>              # compute a delta here using properties we know about the
> @@ -1048,7 +1070,7 @@ class manifest(revlog.revlog):
>              work = heapq.merge([(x, False) for x in added],
>                                 [(x, True) for x in removed])
>
> -            arraytext, deltatext = m.fastdelta(self._fulltextcache[p1], work)
> +            arraytext, deltatext = m.fastdelta(self.fulltextcache[p1], work)
>              cachedelta = self.rev(p1), deltatext
>              text = util.buffer(arraytext)
>              n = self.addrevision(text, transaction, link, p1, p2, cachedelta)
> @@ -1068,7 +1090,7 @@ class manifest(revlog.revlog):
>                  arraytext = array.array('c', text)
>
>          self._mancache[n] = m
> -        self._fulltextcache[n] = arraytext
> +        self.fulltextcache[n] = arraytext
>
>          return n
>
> @@ -1095,6 +1117,6 @@ class manifest(revlog.revlog):
>
>      def clearcaches(self):
>          super(manifest, self).clearcaches()
> -        self._fulltextcache.clear()
> +        super(manifestrevlog, self).clearcaches()

This looks misplaced. Shouldn't it be in manifestrevlog.clearcaches()?

>          self._mancache.clear()
>          self._dirlogcache = {'': self}
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Durham Goode - Aug. 17, 2016, 8:12 p.m.
On 8/11/16 9:06 PM, Martin von Zweigbergk wrote:
> On Mon, Aug 8, 2016 at 6:17 PM, Durham Goode <durham@fb.com> wrote:
>> # HG changeset patch
>> # User Durham Goode <durham@fb.com>
>> # Date 1470696725 25200
>> #      Mon Aug 08 15:52:05 2016 -0700
>> # Node ID 6bcb94d28e6fa3a9429926fec8a42f3f8699b8f5
>> # Parent  f91cdd4315bbc92ad893c8084c0347c218399ce3
>> manifest: make manifest derive from manifestrevlog
>>
>> As part of our refactoring to split the manifest concept from its storage, we
>> need to start moving the revlog specific parts of the manifest implementation to
>> a new class. This patch creates manifestrevlog and moves the fulltextcache onto
>> the base class.
>>
>> diff --git a/mercurial/manifest.py b/mercurial/manifest.py
>> --- a/mercurial/manifest.py
>> +++ b/mercurial/manifest.py
>> @@ -890,7 +890,30 @@ class treemanifest(object):
>>                   subp1, subp2 = subp2, subp1
>>               writesubtree(subm, subp1, subp2)
>>
>> -class manifest(revlog.revlog):
>> +class manifestrevlog(revlog.revlog):
>> +    '''A revlog that stores manifest texts. This is responsible for caching the
>> +    full-text manifest contents.
>> +    '''
>> +    def __init__(self, opener, indexfile):
>> +        super(manifestrevlog, self).__init__(opener, indexfile)
>> +
>> +        # During normal operations, we expect to deal with not more than four
>> +        # revs at a time (such as during commit --amend). When rebasing large
>> +        # stacks of commits, the number can go up, hence the config knob below.
>> +        cachesize = 4
>> +        opts = getattr(opener, 'options', None)
>> +        if opts is not None:
>> +            cachesize = opts.get('manifestcachesize', cachesize)
>> +        self._fulltextcache = util.lrucachedict(cachesize)
>> +
>> +    @property
>> +    def fulltextcache(self):
>> +        return self._fulltextcache
>> +
>> +    def clearcaches(self):
>> +        self._fulltextcache.clear()
>> +
>> +class manifest(manifestrevlog):
>>       def __init__(self, opener, dir='', dirlogcache=None):
>>           '''The 'dir' and 'dirlogcache' arguments are for internal use by
>>           manifest.manifest only. External users should create a root manifest
>> @@ -908,7 +931,6 @@ class manifest(revlog.revlog):
>>               usetreemanifest = opts.get('treemanifest', usetreemanifest)
>>               usemanifestv2 = opts.get('manifestv2', usemanifestv2)
>>           self._mancache = util.lrucachedict(cachesize)
>> -        self._fulltextcache = util.lrucachedict(cachesize)
>>           self._treeinmem = usetreemanifest
>>           self._treeondisk = usetreemanifest
>>           self._usemanifestv2 = usemanifestv2
>> @@ -918,7 +940,7 @@ class manifest(revlog.revlog):
>>               if not dir.endswith('/'):
>>                   dir = dir + '/'
>>               indexfile = "meta/" + dir + "00manifest.i"
>> -        revlog.revlog.__init__(self, opener, indexfile)
>> +        super(manifest, self).__init__(opener, indexfile)
>>           self._dir = dir
>>           # The dirlogcache is kept on the root manifest log
>>           if dir:
>> @@ -1016,7 +1038,7 @@ class manifest(revlog.revlog):
>>               m = self._newmanifest(text)
>>               arraytext = array.array('c', text)
>>           self._mancache[node] = m
>> -        self._fulltextcache[node] = arraytext
>> +        self.fulltextcache[node] = arraytext
>>           return m
>>
>>       def readshallow(self, node):
>> @@ -1036,7 +1058,7 @@ class manifest(revlog.revlog):
>>               return None, None
>>
>>       def add(self, m, transaction, link, p1, p2, added, removed):
>> -        if (p1 in self._fulltextcache and not self._treeinmem
>> +        if (p1 in self.fulltextcache and not self._treeinmem
>>               and not self._usemanifestv2):
>>               # If our first parent is in the manifest cache, we can
>>               # compute a delta here using properties we know about the
>> @@ -1048,7 +1070,7 @@ class manifest(revlog.revlog):
>>               work = heapq.merge([(x, False) for x in added],
>>                                  [(x, True) for x in removed])
>>
>> -            arraytext, deltatext = m.fastdelta(self._fulltextcache[p1], work)
>> +            arraytext, deltatext = m.fastdelta(self.fulltextcache[p1], work)
>>               cachedelta = self.rev(p1), deltatext
>>               text = util.buffer(arraytext)
>>               n = self.addrevision(text, transaction, link, p1, p2, cachedelta)
>> @@ -1068,7 +1090,7 @@ class manifest(revlog.revlog):
>>                   arraytext = array.array('c', text)
>>
>>           self._mancache[n] = m
>> -        self._fulltextcache[n] = arraytext
>> +        self.fulltextcache[n] = arraytext
>>
>>           return n
>>
>> @@ -1095,6 +1117,6 @@ class manifest(revlog.revlog):
>>
>>       def clearcaches(self):
>>           super(manifest, self).clearcaches()
>> -        self._fulltextcache.clear()
>> +        super(manifestrevlog, self).clearcaches()
> This looks misplaced. Shouldn't it be in manifestrevlog.clearcaches()?
You are correct.  Will fix and resend.
>
>>           self._mancache.clear()
>>           self._dirlogcache = {'': self}
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=CwIBaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=nuarHzhP1wi1T9iURRCj1A&m=9l12kRBMMiYL4BgeLJhCQ0FqDtfRuRh7hDevbCs25Wo&s=2Uj4DsjOvYX1jliBfH-6TzKXRdWzOO05kqIoghHdrAk&e=

Patch

diff --git a/mercurial/manifest.py b/mercurial/manifest.py
--- a/mercurial/manifest.py
+++ b/mercurial/manifest.py
@@ -890,7 +890,30 @@  class treemanifest(object):
                 subp1, subp2 = subp2, subp1
             writesubtree(subm, subp1, subp2)
 
-class manifest(revlog.revlog):
+class manifestrevlog(revlog.revlog):
+    '''A revlog that stores manifest texts. This is responsible for caching the
+    full-text manifest contents.
+    '''
+    def __init__(self, opener, indexfile):
+        super(manifestrevlog, self).__init__(opener, indexfile)
+
+        # During normal operations, we expect to deal with not more than four
+        # revs at a time (such as during commit --amend). When rebasing large
+        # stacks of commits, the number can go up, hence the config knob below.
+        cachesize = 4
+        opts = getattr(opener, 'options', None)
+        if opts is not None:
+            cachesize = opts.get('manifestcachesize', cachesize)
+        self._fulltextcache = util.lrucachedict(cachesize)
+
+    @property
+    def fulltextcache(self):
+        return self._fulltextcache
+
+    def clearcaches(self):
+        self._fulltextcache.clear()
+
+class manifest(manifestrevlog):
     def __init__(self, opener, dir='', dirlogcache=None):
         '''The 'dir' and 'dirlogcache' arguments are for internal use by
         manifest.manifest only. External users should create a root manifest
@@ -908,7 +931,6 @@  class manifest(revlog.revlog):
             usetreemanifest = opts.get('treemanifest', usetreemanifest)
             usemanifestv2 = opts.get('manifestv2', usemanifestv2)
         self._mancache = util.lrucachedict(cachesize)
-        self._fulltextcache = util.lrucachedict(cachesize)
         self._treeinmem = usetreemanifest
         self._treeondisk = usetreemanifest
         self._usemanifestv2 = usemanifestv2
@@ -918,7 +940,7 @@  class manifest(revlog.revlog):
             if not dir.endswith('/'):
                 dir = dir + '/'
             indexfile = "meta/" + dir + "00manifest.i"
-        revlog.revlog.__init__(self, opener, indexfile)
+        super(manifest, self).__init__(opener, indexfile)
         self._dir = dir
         # The dirlogcache is kept on the root manifest log
         if dir:
@@ -1016,7 +1038,7 @@  class manifest(revlog.revlog):
             m = self._newmanifest(text)
             arraytext = array.array('c', text)
         self._mancache[node] = m
-        self._fulltextcache[node] = arraytext
+        self.fulltextcache[node] = arraytext
         return m
 
     def readshallow(self, node):
@@ -1036,7 +1058,7 @@  class manifest(revlog.revlog):
             return None, None
 
     def add(self, m, transaction, link, p1, p2, added, removed):
-        if (p1 in self._fulltextcache and not self._treeinmem
+        if (p1 in self.fulltextcache and not self._treeinmem
             and not self._usemanifestv2):
             # If our first parent is in the manifest cache, we can
             # compute a delta here using properties we know about the
@@ -1048,7 +1070,7 @@  class manifest(revlog.revlog):
             work = heapq.merge([(x, False) for x in added],
                                [(x, True) for x in removed])
 
-            arraytext, deltatext = m.fastdelta(self._fulltextcache[p1], work)
+            arraytext, deltatext = m.fastdelta(self.fulltextcache[p1], work)
             cachedelta = self.rev(p1), deltatext
             text = util.buffer(arraytext)
             n = self.addrevision(text, transaction, link, p1, p2, cachedelta)
@@ -1068,7 +1090,7 @@  class manifest(revlog.revlog):
                 arraytext = array.array('c', text)
 
         self._mancache[n] = m
-        self._fulltextcache[n] = arraytext
+        self.fulltextcache[n] = arraytext
 
         return n
 
@@ -1095,6 +1117,6 @@  class manifest(revlog.revlog):
 
     def clearcaches(self):
         super(manifest, self).clearcaches()
-        self._fulltextcache.clear()
+        super(manifestrevlog, self).clearcaches()
         self._mancache.clear()
         self._dirlogcache = {'': self}