Patchwork [1,of,3,v2] context: don't use util.cachefunc due to cycle creation (issue5043)

mail settings
Submitter Gregory Szorc
Date Jan. 17, 2016, 10:18 p.m.
Message ID <471f676f63e429745e5f.1453069120@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/12819/
State Accepted
Commit c183f7b79541560192ab2cc6864308416d137604
Headers show


Gregory Szorc - Jan. 17, 2016, 10:18 p.m.
# HG changeset patch
# User Gregory Szorc <>
# Date 1453061430 28800
#      Sun Jan 17 12:10:30 2016 -0800
# Node ID 471f676f63e429745e5f691201faa91cfd897e83
# Parent  add2ba16430ea5d31ee26e84e1b4c66dc3a6ee15
context: don't use util.cachefunc due to cycle creation (issue5043)

util.cachefunc stores all arguments as the cache key. For filectxfn
functions, the arguments include the memctx instance. This creates a
cycle where memctx._filectxfn references self. This causes a memory

We break the cycle by implementing our own memoizing function that
only uses the path as the cache key. Since each memctx has its own
cache instance, there is no concern about invalid cache hits.


diff --git a/mercurial/ b/mercurial/
--- a/mercurial/
+++ b/mercurial/
@@ -1774,16 +1774,32 @@  class workingcommitctx(workingctx):
     def _changedset(self):
         """Return the set of files changed in this context
         changed = set(self._status.modified)
         return changed
+def makecachingfilectxfn(func):
+    """Create a filectxfn that caches based on the path.
+    We can't use util.cachefunc because it uses all arguments as the cache
+    key and this creates a cycle since the arguments include the repo and
+    memctx.
+    """
+    cache = {}
+    def getfilectx(repo, memctx, path):
+        if path not in cache:
+            cache[path] = func(repo, memctx, path)
+        return cache[path]
+    return getfilectx
 class memctx(committablectx):
     """Use memctx to perform in-memory commits via localrepo.commitctx().
     Revision information is supplied at initialization time while
     related files data and is made available through a callback
     mechanism.  'repo' is the current localrepo, 'parents' is a
     sequence of two parent revisions identifiers (pass None for every
     missing parent), 'text' is the commit message and 'files' lists
@@ -1833,19 +1849,18 @@  class memctx(committablectx):
                 copied = fctx.renamed()
                 if copied:
                     copied = copied[0]
                 return memfilectx(repo, path,,
                                   islink=fctx.islink(), isexec=fctx.isexec(),
                                   copied=copied, memctx=memctx)
             self._filectxfn = getfilectx
-            # "util.cachefunc" reduces invocation of possibly expensive
-            # "filectxfn" for performance (e.g. converting from another VCS)
-            self._filectxfn = util.cachefunc(filectxfn)
+            # memoizing increases performance for e.g. vcs convert scenarios.
+            self._filectxfn = makecachingfilectxfn(filectxfn)
         if extra:
             self._extra = extra.copy()
             self._extra = {}
         if self._extra.get('branch', '') == '':
             self._extra['branch'] = 'default'