changelog: load pending file directly
authorGregory Szorc <gregory.szorc@gmail.com>
Sat, 13 May 2017 16:26:43 -0700
changeset 32292 0ad0d26ff703
parent 32291 bd872f64a8ba
child 32293 ca727147ff9f
changelog: load pending file directly When changelogs are written, a copy of the index (or inline revlog) may be written to an 00changelog.i.a file to facilitate hooks and other processes having access to the pending data before it is finalized. The way it works today, the localrepo class loads the changelog like normal. Then, if it detects a pending transaction, it asks the changelog class to load a pending changelog. The changelog class looks for a 00changelog.i.a file. If it exists, it is loaded and internal data structures on the new revlog class are copied to the original instance. The existing mechanism is inefficient because it loads 2 revlog files. The index, node map, and chunk cache for 00changelog.i are thrown away and replaced by those for 00changelog.i.a. The existing mechanism is also brittle because it is a layering violation to access the data structures being accessed. For example, the code copies the "chunk cache" because for inline revlogs this cache contains the raw revision chunks and allows the original changelog/revlog instance to access revision data for these pending revisions. This whole behavior of course relies on the revlog constructor reading the entirety of an inline revlog into memory and caching it. That's why it is brittle. (I discovered all this as part of modifying behavior of the chunk cache.) This patch streamlines the loading of a pending 00changelog.i.a revlog by doing it directly in the changelog constructor if told to do so. When this code path is active, we no longer load the 00changelog.i file at all. The only negative outcome I see from this change is if loading 00changelog.i was somehow facilitating a role. But I can't imagine what that would be because we throw away its data (the index data structures are replaced and inline revision data is replaced via the chunk cache) and since 00changelog.i.a is a copy of 00changelog.i, file content should be identical, so there should be no meaninful file integrity checking at play. I think this was all just sub-optimal code.
mercurial/changelog.py
mercurial/localrepo.py
--- a/mercurial/changelog.py	Fri Feb 10 16:56:29 2017 -0800
+++ b/mercurial/changelog.py	Sat May 13 16:26:43 2017 -0700
@@ -258,9 +258,23 @@
         return encoding.tolocal(self._text[self._offsets[3] + 2:])
 
 class changelog(revlog.revlog):
-    def __init__(self, opener):
-        revlog.revlog.__init__(self, opener, "00changelog.i",
-                               checkambig=True)
+    def __init__(self, opener, trypending=False):
+        """Load a changelog revlog using an opener.
+
+        If ``trypending`` is true, we attempt to load the index from a
+        ``00changelog.i.a`` file instead of the default ``00changelog.i``.
+        The ``00changelog.i.a`` file contains index (and possibly inline
+        revision) data for a transaction that hasn't been finalized yet.
+        It exists in a separate file to facilitate readers (such as
+        hooks processes) accessing data before a transaction is finalized.
+        """
+        if trypending and opener.exists('00changelog.i.a'):
+            indexfile = '00changelog.i.a'
+        else:
+            indexfile = '00changelog.i'
+
+        revlog.revlog.__init__(self, opener, indexfile, checkambig=True)
+
         if self._initempty:
             # changelogs don't benefit from generaldelta
             self.version &= ~revlog.REVLOGGENERALDELTA
@@ -401,27 +415,6 @@
         # split when we're done
         self.checkinlinesize(tr)
 
-    def readpending(self, file):
-        """read index data from a "pending" file
-
-        During a transaction, the actual changeset data is already stored in the
-        main file, but not yet finalized in the on-disk index. Instead, a
-        "pending" index is written by the transaction logic. If this function
-        is running, we are likely in a subprocess invoked in a hook. The
-        subprocess is informed that it is within a transaction and needs to
-        access its content.
-
-        This function will read all the index data out of the pending file and
-        overwrite the main index."""
-
-        if not self.opener.exists(file):
-            return # no pending data for changelog
-        r = revlog.revlog(self.opener, file)
-        self.index = r.index
-        self.nodemap = r.nodemap
-        self._nodecache = r._nodecache
-        self._chunkcache = r._chunkcache
-
     def _writepending(self, tr):
         "create a file containing the unfinalized state for pretxnchangegroup"
         if self._delaybuf:
--- a/mercurial/localrepo.py	Fri Feb 10 16:56:29 2017 -0800
+++ b/mercurial/localrepo.py	Sat May 13 16:26:43 2017 -0700
@@ -528,10 +528,8 @@
 
     @storecache('00changelog.i')
     def changelog(self):
-        c = changelog.changelog(self.svfs)
-        if txnutil.mayhavepending(self.root):
-            c.readpending('00changelog.i.a')
-        return c
+        return changelog.changelog(self.svfs,
+                                   trypending=txnutil.mayhavepending(self.root))
 
     def _constructmanifest(self):
         # This is a temporary function while we migrate from manifest to