changeset 51780:852bd109dd55

manifest: use `read_any_fast_delta` for tag rev cache computation

This will have the benefit of using the fast path more often, and being (a bit)
less buggy. See inline comment for details.

--- a/mercurial/tags.py	Thu Aug 01 05:37:57 2024 +0200
+++ b/mercurial/tags.py	Thu Aug 01 12:14:40 2024 +0200
@@ -851,25 +851,45 @@
         rev = ctx.rev()
         fnode = None
         cl = self._repo.changelog
+        ml = self._repo.manifestlog
+        mctx = ctx.manifestctx()
+        base_values = {}
         p1rev, p2rev = cl._uncheckedparentrevs(rev)
-        p1node = cl.node(p1rev)
-        p1fnode = self.getfnode(p1node, computemissing=False)
+        m_p1_node, m_p2_node = mctx.parents
+        if p1rev != nullrev:
+            p1_node = cl.node(p1rev)
+            fnode = self.getfnode(p1_node, computemissing=False)
+            # when unknown, fnode is None or False
+            if fnode:
+                p1_manifest_rev = ml.rev(m_p1_node)
+                base_values[p1_manifest_rev] = fnode
         if p2rev != nullrev:
-            # There is some no-merge changeset where p1 is null and p2 is set
-            # Processing them as merge is just slower, but still gives a good
-            # result.
-            p2node = cl.node(p2rev)
-            p2fnode = self.getfnode(p2node, computemissing=False)
-            if p1fnode != p2fnode:
-                # we cannot rely on readfast because we don't know against what
-                # parent the readfast delta is computed
-                p1fnode = None
-        if p1fnode:
-            mctx = ctx.manifestctx()
-            fnode = mctx.readfast().get(b'.hgtags')
+            p2_node = cl.node(p2rev)
+            fnode = self.getfnode(p2_node, computemissing=False)
+            # when unknown, fnode is None or False
+            if fnode:
+                p2_manifest_rev = ml.rev(m_p2_node)
+                base_values[p2_manifest_rev] = fnode
+        # XXX: Beware that using delta to speed things up here is actually
+        # buggy as it will fails to detect a `.hgtags` deletion. That buggy
+        # behavior has been cargo culted from the previous version of this code
+        # as "in practice this seems fine" and not using delta is just too
+        # slow.
+        #
+        # However note that we only consider delta from p1 or p2 because it is
+        # far less likely to have a .hgtags delete in a child than missing from
+        # one branch to another. As the delta chain construction keep being
+        # optimized, it means we will not use delta as often as we could.
+        if base_values:
+            base, m = mctx.read_any_fast_delta(base_values)
+            fnode = m.get(b'.hgtags')
             if fnode is None:
-                fnode = p1fnode
-        if fnode is None:
+                if base is not None:
+                    fnode = base_values[base]
+                else:
+                    # No delta and .hgtags file on this revision.
+                    fnode = self._repo.nullid
+        else:
             # Populate missing entry.
             try:
                 fnode = ctx.filenode(b'.hgtags')