Patchwork manifest: remove _repo from manifestctx objects

login
register
mail settings
Submitter Durham Goode
Date March 2, 2017, 12:57 a.m.
Message ID <b787c417673391589272.1488416239@dev111.prn1.facebook.com>
Download mbox | patch
Permalink /patch/18858/
State Accepted
Delegated to: Martin von Zweigbergk
Headers show

Comments

Durham Goode - March 2, 2017, 12:57 a.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1488415188 28800
#      Wed Mar 01 16:39:48 2017 -0800
# Node ID b787c41767339158927232ec7a9092196e887453
# Parent  0bb3089fe73527c64f1afc40b86ecb8dfe7fd7aa
manifest: remove _repo from manifestctx objects

We were storing the repo on the manifestctx objects so that they could access
the manifestlog via repo.manifestlog, which would refresh the structure if it
became out of date. This caused probems however when we want to have multiple
manifest logs in memory at once, like when transitioning to tree manifest from
flat manifests, since a tree manifest would try to access sub-trees via
repo.manifestlog[node], which was the flat manifest.

The solution is to just not store the repo, and instead store the manifestlog
that created this context. This removes the invalidation when the in memory
manifestlog becomes out of date, but people should probably not be keeping ctx's
around that long anyway.
Yuya Nishihara - March 5, 2017, 5:04 a.m.
On Wed, 1 Mar 2017 16:57:19 -0800, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1488415188 28800
> #      Wed Mar 01 16:39:48 2017 -0800
> # Node ID b787c41767339158927232ec7a9092196e887453
> # Parent  0bb3089fe73527c64f1afc40b86ecb8dfe7fd7aa
> manifest: remove _repo from manifestctx objects
> 
> We were storing the repo on the manifestctx objects so that they could access
> the manifestlog via repo.manifestlog, which would refresh the structure if it
> became out of date. This caused probems however when we want to have multiple
> manifest logs in memory at once, like when transitioning to tree manifest from
> flat manifests, since a tree manifest would try to access sub-trees via
> repo.manifestlog[node], which was the flat manifest.
> 
> The solution is to just not store the repo, and instead store the manifestlog
> that created this context. This removes the invalidation when the in memory
> manifestlog becomes out of date, but people should probably not be keeping ctx's
> around that long anyway.

The direction seems good and I like it. Queued, thanks.

Patch

diff --git a/mercurial/manifest.py b/mercurial/manifest.py
--- a/mercurial/manifest.py
+++ b/mercurial/manifest.py
@@ -1252,8 +1252,6 @@  class manifestlog(object):
     class do not care about the implementation details of the actual manifests
     they receive (i.e. tree or flat or lazily loaded, etc)."""
     def __init__(self, opener, repo):
-        self._repo = repo
-
         usetreemanifest = False
         cachesize = 4
 
@@ -1300,7 +1298,7 @@  class manifestlog(object):
                     if node not in dirlog.nodemap:
                         raise LookupError(node, dirlog.indexfile,
                                           _('no node'))
-                m = treemanifestctx(self._repo, dir, node)
+                m = treemanifestctx(self, dir, node)
             else:
                 raise error.Abort(
                         _("cannot ask for manifest directory '%s' in a flat "
@@ -1311,9 +1309,9 @@  class manifestlog(object):
                     raise LookupError(node, self._revlog.indexfile,
                                       _('no node'))
             if self._treeinmem:
-                m = treemanifestctx(self._repo, '', node)
+                m = treemanifestctx(self, '', node)
             else:
-                m = manifestctx(self._repo, node)
+                m = manifestctx(self, node)
 
         if node != revlog.nullid:
             mancache = self._dirmancache.get(dir)
@@ -1328,18 +1326,18 @@  class manifestlog(object):
         self._revlog.clearcaches()
 
 class memmanifestctx(object):
-    def __init__(self, repo):
-        self._repo = repo
+    def __init__(self, manifestlog):
+        self._manifestlog = manifestlog
         self._manifestdict = manifestdict()
 
     def _revlog(self):
-        return self._repo.manifestlog._revlog
+        return self._manifestlog._revlog
 
     def new(self):
-        return memmanifestctx(self._repo)
+        return memmanifestctx(self._manifestlog)
 
     def copy(self):
-        memmf = memmanifestctx(self._repo)
+        memmf = memmanifestctx(self._manifestlog)
         memmf._manifestdict = self.read().copy()
         return memmf
 
@@ -1354,8 +1352,8 @@  class manifestctx(object):
     """A class representing a single revision of a manifest, including its
     contents, its parent revs, and its linkrev.
     """
-    def __init__(self, repo, node):
-        self._repo = repo
+    def __init__(self, manifestlog, node):
+        self._manifestlog = manifestlog
         self._data = None
 
         self._node = node
@@ -1368,16 +1366,16 @@  class manifestctx(object):
         #self.linkrev = revlog.linkrev(rev)
 
     def _revlog(self):
-        return self._repo.manifestlog._revlog
+        return self._manifestlog._revlog
 
     def node(self):
         return self._node
 
     def new(self):
-        return memmanifestctx(self._repo)
+        return memmanifestctx(self._manifestlog)
 
     def copy(self):
-        memmf = memmanifestctx(self._repo)
+        memmf = memmanifestctx(self._manifestlog)
         memmf._manifestdict = self.read().copy()
         return memmf
 
@@ -1422,7 +1420,7 @@  class manifestctx(object):
         if revlog._usemanifestv2:
             # Need to perform a slow delta
             r0 = revlog.deltaparent(revlog.rev(self._node))
-            m0 = self._repo.manifestlog[revlog.node(r0)].read()
+            m0 = self._manifestlog[revlog.node(r0)].read()
             m1 = self.read()
             md = manifestdict()
             for f, ((n0, fl0), (n1, fl1)) in m0.diff(m1).iteritems():
@@ -1440,19 +1438,19 @@  class manifestctx(object):
         return self.read().find(key)
 
 class memtreemanifestctx(object):
-    def __init__(self, repo, dir=''):
-        self._repo = repo
+    def __init__(self, manifestlog, dir=''):
+        self._manifestlog = manifestlog
         self._dir = dir
         self._treemanifest = treemanifest()
 
     def _revlog(self):
-        return self._repo.manifestlog._revlog
+        return self._manifestlog._revlog
 
     def new(self, dir=''):
-        return memtreemanifestctx(self._repo, dir=dir)
+        return memtreemanifestctx(self._manifestlog, dir=dir)
 
     def copy(self):
-        memmf = memtreemanifestctx(self._repo, dir=self._dir)
+        memmf = memtreemanifestctx(self._manifestlog, dir=self._dir)
         memmf._treemanifest = self._treemanifest.copy()
         return memmf
 
@@ -1461,13 +1459,13 @@  class memtreemanifestctx(object):
 
     def write(self, transaction, link, p1, p2, added, removed):
         def readtree(dir, node):
-            return self._repo.manifestlog.get(dir, node).read()
+            return self._manifestlog.get(dir, node).read()
         return self._revlog().add(self._treemanifest, transaction, link, p1, p2,
                                   added, removed, readtree=readtree)
 
 class treemanifestctx(object):
-    def __init__(self, repo, dir, node):
-        self._repo = repo
+    def __init__(self, manifestlog, dir, node):
+        self._manifestlog = manifestlog
         self._dir = dir
         self._data = None
 
@@ -1481,7 +1479,7 @@  class treemanifestctx(object):
         #self.linkrev = revlog.linkrev(rev)
 
     def _revlog(self):
-        return self._repo.manifestlog._revlog.dirlog(self._dir)
+        return self._manifestlog._revlog.dirlog(self._dir)
 
     def read(self):
         if self._data is None:
@@ -1495,7 +1493,7 @@  class treemanifestctx(object):
                 def readsubtree(dir, subm):
                     # Set verify to False since we need to be able to create
                     # subtrees for trees that don't exist on disk.
-                    return self._repo.manifestlog.get(dir, subm,
+                    return self._manifestlog.get(dir, subm,
                                                       verify=False).read()
                 m.read(gettext, readsubtree)
                 m.setnode(self._node)
@@ -1512,10 +1510,10 @@  class treemanifestctx(object):
         return self._node
 
     def new(self, dir=''):
-        return memtreemanifestctx(self._repo, dir=dir)
+        return memtreemanifestctx(self._manifestlog, dir=dir)
 
     def copy(self):
-        memmf = memtreemanifestctx(self._repo, dir=self._dir)
+        memmf = memtreemanifestctx(self._manifestlog, dir=self._dir)
         memmf._treemanifest = self.read().copy()
         return memmf
 
@@ -1542,7 +1540,7 @@  class treemanifestctx(object):
         else:
             # Need to perform a slow delta
             r0 = revlog.deltaparent(revlog.rev(self._node))
-            m0 = self._repo.manifestlog.get(self._dir, revlog.node(r0)).read()
+            m0 = self._manifestlog.get(self._dir, revlog.node(r0)).read()
             m1 = self.read()
             md = treemanifest(dir=self._dir)
             for f, ((n0, fl0), (n1, fl1)) in m0.diff(m1).iteritems():