commit: set whole manifest entries at once (node with its associated flags)
authorArseniy Alekseyev <aalekseyev@janestreet.com>
Thu, 01 Aug 2024 13:38:31 +0100
changeset 51758 421c9b3f2f4e
parent 51757 a1e4fa9330d8
child 51761 812a094a7477
commit: set whole manifest entries at once (node with its associated flags) Add a new function manifest.set that sets whole manifest entries at once, so the caller doesn't have to do two separate operations: m[p] = n m.set_flags(f) becomes: m.set(p, n, f) This obviously saves an extra lookup by path, and it also lets the underlying manifest implementation to be more efficient as it doesn't have to deal with partially-specified entries. It makes the interaction conceptually simpler, as well, since we don't have to go through an intermediate state of incorrect partially-written entry. (the real motivation for this change is an alternative manifest implementation where we batch pending writes, and dealing with fully defined entries makes the batching logic muchsimpler while avoiding slowdown due to alternating writes and reads)
mercurial/commit.py
mercurial/interfaces/repository.py
mercurial/manifest.py
--- a/mercurial/commit.py	Thu Aug 01 11:43:10 2024 -0400
+++ b/mercurial/commit.py	Thu Aug 01 13:38:31 2024 +0100
@@ -214,15 +214,15 @@
             elif narrow_action == mergestate.CHANGE_ADDED:
                 files.mark_added(f)
                 added.append(f)
-                m[f] = m2[f]
+                fnode = m2[f]
                 flags = m2ctx.find(f)[1] or b''
-                m.setflag(f, flags)
+                m.set(f, fnode, flags)
             elif narrow_action == mergestate.CHANGE_MODIFIED:
                 files.mark_touched(f)
                 added.append(f)
-                m[f] = m2[f]
+                fnode = m2[f]
                 flags = m2ctx.find(f)[1] or b''
-                m.setflag(f, flags)
+                m.set(f, fnode, flags)
             else:
                 msg = _(b"corrupted mergestate, unknown narrow action: %b")
                 hint = _(b"restart the merge")
@@ -234,7 +234,7 @@
                 removed.append(f)
             else:
                 added.append(f)
-                m[f], is_touched = _filecommit(
+                fnode, is_touched = _filecommit(
                     repo, fctx, m1, m2, linkrev, tr, writefilecopymeta, ms
                 )
                 if is_touched:
@@ -244,7 +244,7 @@
                         files.mark_merged(f)
                     else:
                         files.mark_touched(f)
-                m.setflag(f, fctx.flags())
+                m.set(f, fnode, fctx.flags())
         except OSError:
             repo.ui.warn(_(b"trouble committing %s!\n") % uipathfn(f))
             raise
--- a/mercurial/interfaces/repository.py	Thu Aug 01 11:43:10 2024 -0400
+++ b/mercurial/interfaces/repository.py	Thu Aug 01 13:38:31 2024 +0100
@@ -1021,6 +1021,12 @@
 
     __bool__ = __nonzero__
 
+    def set(path, node, flags):
+        """Define the node value and flags for a path in the manifest.
+
+        Equivalent to __setitem__ followed by setflag, but can be more efficient.
+        """
+
     def __setitem__(path, node):
         """Define the node value for a path in the manifest.
 
--- a/mercurial/manifest.py	Thu Aug 01 11:43:10 2024 -0400
+++ b/mercurial/manifest.py	Thu Aug 01 13:38:31 2024 +0100
@@ -493,6 +493,9 @@
 
     __bool__ = __nonzero__
 
+    def set(self, key, node, flags):
+        self._lm[key] = node, flags
+
     def __setitem__(self, key, node):
         self._lm[key] = node, self.flags(key)
 
@@ -1048,6 +1051,26 @@
                 del self._flags[f]
         self._dirty = True
 
+    def set(self, f, node, flags):
+        """Set both the node and the flags for path f."""
+        assert node is not None
+        if flags not in _manifestflags:
+            raise TypeError(b"Invalid manifest flag set.")
+        self._load()
+        dir, subpath = _splittopdir(f)
+        if dir:
+            self._loadlazy(dir)
+            if dir not in self._dirs:
+                self._dirs[dir] = treemanifest(
+                    self.nodeconstants, self._subpath(dir)
+                )
+            self._dirs[dir].set(subpath, node, flags)
+        else:
+            assert len(node) in (20, 32)
+            self._files[f] = node
+            self._flags[f] = flags
+        self._dirty = True
+
     def __setitem__(self, f, n):
         assert n is not None
         self._load()