Patchwork [8,of,8] annotate: simplify annotate parent function

login
register
mail settings
Submitter Durham Goode
Date May 31, 2013, 5:19 p.m.
Message ID <73654634246ac92ee614.1370020790@dev350.prn1.facebook.com>
Download mbox | patch
Permalink /patch/1691/
State Superseded
Commit e0aa6fff8f0241a0b655a15a3856b9ddf8bb0cb3
Headers show

Comments

Durham Goode - May 31, 2013, 5:19 p.m.
# HG changeset patch
# User Durham Goode <durham@fb.com>
# Date 1369967343 25200
#      Thu May 30 19:29:03 2013 -0700
# Node ID 73654634246ac92ee61438eea2d50aab8565c75e
# Parent  7578ad45e1d323e90abd12ad024939e9703aa882
annotate: simplify annotate parent function

The annotate algorithm used a custom parents() function to try to reuse
filectx and filelogs. I got rid of all the caching bits and just use the
normal filectx.parents() and found that the performance did not change
when running hg blame on a file with 5000 revisions (22 seconds before
and after my change).

A side effect of this change is that it also gets rid of the dependency
on filelog revision numbers, which makes this algorithm work with
alternative filelog implementations.
Matt Mackall - June 3, 2013, 8:50 p.m.
On Fri, 2013-05-31 at 10:19 -0700, Durham Goode wrote:
> # HG changeset patch
> # User Durham Goode <durham@fb.com>
> # Date 1369967343 25200
> #      Thu May 30 19:29:03 2013 -0700
> # Node ID 73654634246ac92ee61438eea2d50aab8565c75e
> # Parent  7578ad45e1d323e90abd12ad024939e9703aa882
> annotate: simplify annotate parent function
> 
> The annotate algorithm used a custom parents() function to try to reuse
> filectx and filelogs. I got rid of all the caching bits and just use the
> normal filectx.parents() and found that the performance did not change
> when running hg blame on a file with 5000 revisions (22 seconds before
> and after my change).

The existing logic is to avoid bad behavior on files that have lots of
merges and renames in their history. If your test isn't switching back
and forth between different filelogs as it goes, it doesn't tell us
much.

Patch

diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -648,26 +648,14 @@ 
                     child[0][b1:b2] = parent[0][a1:a2]
             return child
 
-        getlog = util.lrucachefunc(lambda x: self._repo.file(x))
-        def getctx(path, fileid):
-            log = path == self._path and self._filelog or getlog(path)
-            return filectx(self._repo, path, fileid=fileid, filelog=log)
-        getctx = util.lrucachefunc(getctx)
+        def parents(f):
+            pl = f.parents()
 
-        def parents(f):
-            # we want to reuse filectx objects as much as possible
-            p = f._path
-            if f._filerev is None: # working dir
-                pl = [(n.path(), n.filerev()) for n in f.parents()]
-            else:
-                pl = [(p, n) for n in f._filelog.parentrevs(f._filerev)]
+            # Don't return renamed parents if we aren't following.
+            if not follow:
+                pl = [p for p in pl if p.path() == f.path()]
 
-            if follow:
-                r = f.renamed()
-                if r:
-                    pl[0] = (r[0], getlog(r[0]).rev(r[1]))
-
-            return [getctx(p, n) for p, n in pl if n != nullrev]
+            return pl
 
         # use linkrev to find the first changeset where self appeared
         if self.rev() != self.linkrev():