remotefilelog: add a pendingmutablepack class

Summary:
The code to handle pending mutable packs was effectively duplicated in 3
places. The new class allows refactoring of all of it. The use of several
lambdas is unfortunate, but required as the repo and the cache path are dynamic
and can't be obtained when constructing the pendingmutablepack object.

Reviewed By: DurhamG

Differential Revision: D15023059

fbshipit-source-id: 1eae68fe66ce741eb36baa0c8db318ba32957b41

edenscm/hgext/remotefilelog/mutablestores.py

@@ -5,10 +5,65 @@
 
 from edenscm.mercurial.node import hex
 
+from .datapack import mutabledatapack
+from .historypack import mutablehistorypack
 
-# Offsets in the mutable pack tuple
-DATA = 0
-HISTORY = 1
+
+class pendingmutablepack(object):
+    def __init__(self, repo, pathcb):
+        self._mutabledpack = None
+        self._mutablehpack = None
+
+        self._repo = repo
+        self._pathcb = pathcb
+
+    def getmutabledpack(self, read=False):
+        if self._mutabledpack is None and not read:
+            path = self._pathcb()
+            self._mutabledpack = mutabledatapack(self._repo.ui, path)
+
+        return self._mutabledpack
+
+    def getmutablehpack(self, read=False):
+        if self._mutablehpack is None and not read:
+            path = self._pathcb()
+            self._mutablehpack = mutablehistorypack(
+                self._repo.ui, path, repo=self._repo
+            )
+
+        return self._mutablehpack
+
+    def getmutablepack(self):
+        dpack = self.getmutabledpack()
+        hpack = self.getmutablehpack()
+
+        return dpack, hpack
+
+    def commit(self):
+        if self._mutabledpack is not None:
+            try:
+                self._mutabledpack.close()
+            finally:
+                self._mutabledpack = None
+
+        if self._mutablehpack is not None:
+            try:
+                self._mutablehpack.close()
+            finally:
+                self._mutablehpack = None
+
+    def abort(self):
+        if self._mutabledpack is not None:
+            try:
+                self._mutabledpack.abort()
+            finally:
+                self._mutabledpack = None
+
+        if self._mutablehpack is not None:
+            try:
+                self._mutablehpack.abort()
+            finally:
+                self._mutablehpack = None
 
 
 class mutabledatahistorystore(object):
@@ -17,64 +72,57 @@ class mutabledatahistorystore(object):
     insert the mutable packs themselves into the union store because they can be
     created and destroyed over time."""
 
-    def __init__(self, log, shared=False):
-        self.log = log
-        self.shared = shared
-
-    def _packs(self):
-        if self.shared:
-            return self.log._mutablesharedpacks
-        else:
-            return self.log._mutablelocalpacks
+    def __init__(self, getpendingpacks):
+        self.getpendingpacks = getpendingpacks
 
     def getmissing(self, keys):
-        packs = self._packs()
-        if packs is None:
+        dpack = self.getpendingpacks().getmutabledpack(True)
+        if dpack is None:
             return keys
 
-        return packs[DATA].getmissing(keys)
+        return dpack.getmissing(keys)
 
     def get(self, name, node):
-        packs = self._packs()
-        if packs is None:
+        dpack = self.getpendingpacks().getmutabledpack(True)
+        if dpack is None:
             raise KeyError(name, hex(node))
 
-        return packs[DATA].get(name, node)
+        return dpack.get(name, node)
 
     def getdelta(self, name, node):
-        packs = self._packs()
-        if packs is None:
+        dpack = self.getpendingpacks().getmutabledpack(True)
+        if dpack is None:
             raise KeyError(name, hex(node))
 
-        return packs[DATA].getdelta(name, node)
+        return dpack.getdelta(name, node)
 
     def getdeltachain(self, name, node):
-        packs = self._packs()
-        if packs is None:
+        dpack = self.getpendingpacks().getmutabledpack(True)
+        if dpack is None:
             raise KeyError(name, hex(node))
 
-        return packs[DATA].getdeltachain(name, node)
+        return dpack.getdeltachain(name, node)
 
     def getmeta(self, name, node):
-        packs = self._packs()
-        if packs is None:
+        dpack = self.getpendingpacks().getmutabledpack(True)
+        if dpack is None:
             raise KeyError(name, hex(node))
 
-        return packs[DATA].getmeta(name, node)
+        return dpack.getmeta(name, node)
 
     def getnodeinfo(self, name, node):
-        packs = self._packs()
-        if packs is None:
+        hpack = self.getpendingpacks().getmutablehpack(True)
+        if hpack is None:
             raise KeyError(name, hex(node))
 
-        return packs[HISTORY].getnodeinfo(name, node)
+        return hpack.getnodeinfo(name, node)
 
     def getancestors(self, name, node, known=None):
-        packs = self._packs()
-        if packs is None:
+        hpack = self.getpendingpacks().getmutablehpack(True)
+        if hpack is None:
             raise KeyError(name, hex(node))
 
-        return packs[HISTORY].getancestors(name, node, known=known)
+        return hpack.getancestors(name, node, known=known)
 
     def getmetrics(self):
         return {}

edenscm/hgext/remotefilelog/remotefilelog.py

@@ -21,8 +21,8 @@ from .contentstore import (
     remotefilelogcontentstore,
     unioncontentstore,
 )
-from .datapack import datapackstore, mutabledatapack
-from .historypack import historypackstore, mutablehistorypack
+from .datapack import datapackstore
+from .historypack import historypackstore
 from .metadatastore import (
     remotefilelogmetadatastore,
     remotemetadatastore,
@@ -507,44 +507,31 @@ class remotefileslog(filelog.fileslog):
 
     def __init__(self, repo):
         super(remotefileslog, self).__init__(repo)
+        self._mutablelocalpacks = mutablestores.pendingmutablepack(
+            repo,
+            lambda: shallowutil.getlocalpackpath(
+                self.repo.svfs.vfs.base, constants.FILEPACK_CATEGORY
+            ),
+        )
         self.makeunionstores()
-        self._mutablelocalpacks = None
 
     def getmutablelocalpacks(self):
-        if self._mutablelocalpacks is None:
-            packpath = shallowutil.getlocalpackpath(
-                self.repo.svfs.vfs.base, constants.FILEPACK_CATEGORY
-            )
-            self._mutablelocalpacks = (
-                mutabledatapack(self.ui, packpath),
-                mutablehistorypack(self.ui, packpath, repo=self.repo),
-            )
-        return self._mutablelocalpacks
+        return self._mutablelocalpacks.getmutablepack()
 
     def commitpending(self):
         """Used in alternative filelog implementations to commit pending
         additions."""
-        if self._mutablelocalpacks is not None:
-            dpack, hpack = self._mutablelocalpacks
-            self._mutablelocalpacks = None
+        self._mutablelocalpacks.commit()
 
-            dpack.close()
-            hpack.close()
-
-            for store in self.localdatastores:
-                store.markforrefresh()
-            for store in self.localhistorystores:
-                store.markforrefresh()
+        for store in self.localdatastores:
+            store.markforrefresh()
+        for store in self.localhistorystores:
+            store.markforrefresh()
 
     def abortpending(self):
         """Used in alternative filelog implementations to throw out pending
         additions."""
-        if self._mutablelocalpacks is not None:
-            dpack, hpack = self._mutablelocalpacks
-            self._mutablelocalpacks = None
-
-            dpack.abort()
-            hpack.abort()
+        self._mutablelocalpacks.abort()
 
     def makeunionstores(self):
         """Union stores iterate the other stores and return the first result."""
@@ -576,7 +563,9 @@ class remotefileslog(filelog.fileslog):
                 cachecontent, cachemetadata
             )
 
-        mutablelocalstore = mutablestores.mutabledatahistorystore(self)
+        mutablelocalstore = mutablestores.mutabledatahistorystore(
+            lambda: self._mutablelocalpacks
+        )
 
         # Instantiate union stores
         self.contentstore = unioncontentstore(

edenscm/hgext/treemanifest/__init__.py

@@ -511,7 +511,9 @@ def setuptreestores(repo, mfl):
     if ui.configbool("treemanifest", "server"):
         packpath = repo.localvfs.join("cache/packs/%s" % PACK_CATEGORY)
 
-        mutablelocalstore = mutablestores.mutabledatahistorystore(mfl)
+        mutablelocalstore = mutablestores.mutabledatahistorystore(
+            lambda: mfl._mutablelocalpacks
+        )
         ondemandstore = ondemandtreedatastore(repo)
 
         # Data store
@@ -581,8 +583,12 @@ def setuptreestores(repo, mfl):
     remotestore = remotetreestore(repo)
     ondemandstore = ondemandtreedatastore(repo)
 
-    mutablelocalstore = mutablestores.mutabledatahistorystore(mfl)
-    mutablesharedstore = mutablestores.mutabledatahistorystore(mfl, shared=True)
+    mutablelocalstore = mutablestores.mutabledatahistorystore(
+        lambda: mfl._mutablelocalpacks
+    )
+    mutablesharedstore = mutablestores.mutabledatahistorystore(
+        lambda: mfl._mutablesharedpacks
+    )
 
     # Data store
     # TODO: support cstore.uniondatapackstore here
@@ -647,9 +653,14 @@ def setuptreestores(repo, mfl):
 
 
 class basetreemanifestlog(object):
-    def __init__(self):
-        self._mutablelocalpacks = None
-        self._mutablesharedpacks = None
+    def __init__(self, repo):
+        self._mutablelocalpacks = mutablestores.pendingmutablepack(
+            repo,
+            lambda: shallowutil.getlocalpackpath(self._opener.vfs.base, "manifests"),
+        )
+        self._mutablesharedpacks = mutablestores.pendingmutablepack(
+            repo, lambda: shallowutil.getcachepackpath(self._repo, PACK_CATEGORY)
+        )
         self.recentlinknode = None
 
     def add(
@@ -695,22 +706,10 @@ class basetreemanifestlog(object):
 
     def _getmutablelocalpacks(self):
         """Returns a tuple containing a data pack and a history pack."""
-        if self._mutablelocalpacks is None:
-            packpath = shallowutil.getlocalpackpath(self._opener.vfs.base, "manifests")
-            self._mutablelocalpacks = (
-                mutabledatapack(self.ui, packpath),
-                mutablehistorypack(self.ui, packpath, repo=self._repo),
-            )
-        return self._mutablelocalpacks
+        return self._mutablelocalpacks.getmutablepack()
 
     def getmutablesharedpacks(self):
-        if self._mutablesharedpacks is None:
-            packpath = shallowutil.getcachepackpath(self._repo, PACK_CATEGORY)
-            self._mutablesharedpacks = (
-                mutabledatapack(self.ui, packpath),
-                mutablehistorypack(self.ui, packpath, repo=self._repo),
-            )
-        return self._mutablesharedpacks
+        return self._mutablesharedpacks.getmutablepack()
 
     def _addtopack(
         self,
@@ -801,40 +800,15 @@ class basetreemanifestlog(object):
         return node
 
     def commitpending(self):
-        if self._mutablelocalpacks is not None:
-            dpack, hpack = self._mutablelocalpacks
-            self._mutablelocalpacks = None
+        self._mutablelocalpacks.commit()
+        self._mutablesharedpacks.commit()
 
-            dpack.close()
-            hpack.close()
-
-            self.datastore.markforrefresh()
-            self.historystore.markforrefresh()
-
-        if self._mutablesharedpacks is not None:
-            dpack, hpack = self._mutablesharedpacks
-            self._mutablesharedpacks = None
-
-            dpack.close()
-            hpack.close()
-
-            self.datastore.markforrefresh()
-            self.historystore.markforrefresh()
+        self.datastore.markforrefresh()
+        self.historystore.markforrefresh()
 
     def abortpending(self):
-        if self._mutablelocalpacks is not None:
-            dpack, hpack = self._mutablelocalpacks
-            self._mutablelocalpacks = None
-
-            dpack.abort()
-            hpack.abort()
-
-        if self._mutablesharedpacks is not None:
-            dpack, hpack = self._mutablesharedpacks
-            self._mutablesharedpacks = None
-
-            dpack.abort()
-            hpack.abort()
+        self._mutablelocalpacks.abort()
+        self._mutablesharedpacks.abort()
 
     def __nonzero__(self):
         return True
@@ -863,7 +837,8 @@ class basetreemanifestlog(object):
 
 class treemanifestlog(basetreemanifestlog, manifest.manifestlog):
     def __init__(self, opener, repo, treemanifest=False):
-        basetreemanifestlog.__init__(self)
+        self._repo = repo.unfiltered()
+        basetreemanifestlog.__init__(self, self._repo)
         assert treemanifest is False
         cachesize = 4
 
@@ -874,8 +849,6 @@ class treemanifestlog(basetreemanifestlog, manifest.manifestlog):
             cachesize = opts.get("manifestcachesize", cachesize)
         self._treeinmem = True
 
-        self._repo = repo.unfiltered()
-
         self._opener = opener
 
         # A cache of the manifestctx or treemanifestctx for each directory
@@ -891,10 +864,10 @@ class treemanifestlog(basetreemanifestlog, manifest.manifestlog):
 
 class treeonlymanifestlog(basetreemanifestlog):
     def __init__(self, opener, repo):
-        super(treeonlymanifestlog, self).__init__()
+        self._repo = repo.unfiltered()
+        super(treeonlymanifestlog, self).__init__(self._repo)
         self._opener = opener
         self.ui = repo.ui
-        self._repo = repo.unfiltered()
 
     def clearcaches(self):
         pass
@@ -1128,6 +1101,27 @@ def getbundlemanifestlog(orig, self):
     if isinstance(mfl, hybridmanifestlog):
         wrapmfl = mfl.treemanifestlog
 
+    class pendingmempack(object):
+        def __init__(self):
+            self._mutabledpack = None
+            self._mutablehpack = None
+
+        def getmutabledpack(self, read=False):
+            if self._mutabledpack is None and not read:
+                self._mutabledpack = memdatapack()
+            return self._mutabledpack
+
+        def getmutablehpack(self, read=False):
+            if self._mutablehpack is None and not read:
+                self._mutablehpack = memhistorypack()
+            return self._mutablehpack
+
+        def getmutablepack(self):
+            dpack = self.getmutabledpack()
+            hpack = self.getmutablehpack()
+
+            return dpack, hpack
+
     class bundlemanifestlog(wrapmfl.__class__):
         def add(
             self,
@@ -1152,16 +1146,6 @@ def getbundlemanifestlog(orig, self):
                 linkrev=linkrev,
             )
 
-        def _getmutablelocalpacks(self):
-            if self._mutablelocalpacks is None:
-                self._mutablelocalpacks = (memdatapack(), memhistorypack())
-            return self._mutablelocalpacks
-
-        def getmutablesharedpacks(self):
-            if self._mutablesharedpacks is None:
-                self._mutablesharedpacks = (memdatapack(), memhistorypack())
-            return self._mutablesharedpacks
-
         def commitpending(self):
             pass
 
@@ -1170,6 +1154,8 @@ def getbundlemanifestlog(orig, self):
             self._mutablesharedpacks = None
 
     wrapmfl.__class__ = bundlemanifestlog
+    wrapmfl._mutablelocalpacks = pendingmempack()
+    wrapmfl._mutablesharedpacks = pendingmempack()
     return mfl