D9277: transaction: change list of journal entries into a dictionary

[joerg.sonnenberger (Joerg Sonnenberger)]

joerg.sonnenberger created this revision.
Herald added a reviewer: hg-reviewers.
Herald added a subscriber: mercurial-patches.

REVISION SUMMARY
  The transaction object used to keep a mapping table of path names to
  journal entries and a list of journal entries consisting of path and
  file offset to truncate on rollback. Code normally doesn't care about
  the order at all with the exception of repair.strip, which cares only
  about finding new entries. For the special case of repair.strip,
  remember the keys before the strip operation and compute the difference
  with the keys afterwards. Some extra care is necessary as the test case
  wants deterministic operation, but this is outside the hot path so just
  sort everything.

REPOSITORY
  rHG Mercurial

BRANCH
  default

REVISION DETAIL
  https://phab.mercurial-scm.org/D9277

AFFECTED FILES
  mercurial/repair.py
  mercurial/transaction.py

CHANGE DETAILS

diff --git a/mercurial/transaction.py b/mercurial/transaction.py
--- a/mercurial/transaction.py
+++ b/mercurial/transaction.py
@@ -158,8 +158,7 @@
         vfsmap[b''] = opener  # set default value
         self._vfsmap = vfsmap
         self._after = after
-        self._entries = []
-        self._map = {}
+        self._offsetmap = {}
         self._journal = journalname
         self._undoname = undoname
         self._queue = []
@@ -249,7 +248,7 @@
     @active
     def add(self, file, offset):
         """record the state of an append-only file before update"""
-        if file in self._map or file in self._backupmap:
+        if file in self._offsetmap or file in self._backupmap:
             return
         if self._queue:
             self._queue[-1].append((file, offset))
@@ -259,10 +258,9 @@
 
     def _addentry(self, file, offset):
         """add a append-only entry to memory and on-disk state"""
-        if file in self._map or file in self._backupmap:
+        if file in self._offsetmap or file in self._backupmap:
             return
-        self._entries.append((file, offset))
-        self._map[file] = len(self._entries) - 1
+        self._offsetmap[file] = offset
         # add enough data to the journal to do the truncate
         self._file.write(b"%s\0%d\n" % (file, offset))
         self._file.flush()
@@ -282,7 +280,7 @@
             msg = b'cannot use transaction.addbackup inside "group"'
             raise error.ProgrammingError(msg)
 
-        if file in self._map or file in self._backupmap:
+        if file in self._offsetmap or file in self._backupmap:
             return
         vfs = self._vfsmap[location]
         dirname, filename = vfs.split(file)
@@ -396,9 +394,7 @@
 
     @active
     def findoffset(self, file):
-        if file in self._map:
-            return self._entries[self._map[file]][1]
-        return None
+        return self._offsetmap.get(file)
 
     @active
     def replace(self, file, offset):
@@ -407,10 +403,9 @@
         that are not pending in the queue
         '''
 
-        if file not in self._map:
+        if file not in self._offsetmap:
             raise KeyError(file)
-        index = self._map[file]
-        self._entries[index] = (file, offset)
+        self._offsetmap[file] = offset
         self._file.write(b"%s\0%d\n" % (file, offset))
         self._file.flush()
 
@@ -550,7 +545,7 @@
                     self._report(
                         b"couldn't remove %s: %s\n" % (vfs.join(b), inst)
                     )
-        self._entries = []
+        self._offsetmap = {}
         self._writeundo()
         if self._after:
             self._after()
@@ -631,9 +626,11 @@
         self._usages = 0
         self._file.close()
         self._backupsfile.close()
+        entries = list(pycompat.iteritems(self._offsetmap))
+        entries.sort()
 
         try:
-            if not self._entries and not self._backupentries:
+            if not self._offsetmap and not self._backupentries:
                 if self._backupjournal:
                     self._opener.unlink(self._backupjournal)
                 if self._journal:
@@ -652,7 +649,7 @@
                     self._report,
                     self._opener,
                     self._vfsmap,
-                    self._entries,
+                    entries,
                     self._backupentries,
                     False,
                     checkambigfiles=self._checkambigfiles,
diff --git a/mercurial/repair.py b/mercurial/repair.py
--- a/mercurial/repair.py
+++ b/mercurial/repair.py
@@ -209,7 +209,7 @@
                 # transaction and makes assumptions that file storage is
                 # using append-only files. We'll need some kind of storage
                 # API to handle stripping for us.
-                offset = len(tr._entries)
+                oldfiles = set(tr._offsetmap.keys())
 
                 tr.startgroup()
                 cl.strip(striprev, tr)
@@ -219,8 +219,14 @@
                     repo.file(fn).strip(striprev, tr)
                 tr.endgroup()
 
-                for i in pycompat.xrange(offset, len(tr._entries)):
-                    file, troffset = tr._entries[i]
+                newfiles = set(tr._offsetmap.keys())
+                newfiles.difference_update(oldfiles)
+
+                # The processing order doesn't matter during normal
+                # operation, but the test-repair-strip.t test case
+                # inserts faults and it benefits from the sorting.
+                for file in sorted(newfiles):
+                    troffset = tr.findoffset(file)
                     with repo.svfs(file, b'a', checkambig=True) as fp:
                         fp.truncate(troffset)
                     if troffset == 0:



To: joerg.sonnenberger, #hg-reviewers
Cc: mercurial-patches, mercurial-devel