[PATCH 2 of 5] manifest: make manifest derive from manifestrevlog

[Durham Goode]

On 8/11/16 9:06 PM, Martin von Zweigbergk wrote:
> On Mon, Aug 8, 2016 at 6:17 PM, Durham Goode <durham at fb.com> wrote:
>> # HG changeset patch
>> # User Durham Goode <durham at 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 at 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=