From patchwork Sat Nov 7 21:32:17 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: D9277: transaction: change list of journal entries into a dictionary From: phabricator X-Patchwork-Id: 47550 Message-Id: To: Phabricator Cc: mercurial-devel@mercurial-scm.org Date: Sat, 7 Nov 2020 21:32:17 +0000 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 To: joerg.sonnenberger, #hg-reviewers Cc: mercurial-patches, mercurial-devel 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: