[PATCH 1 of 4] pycompat: name maplist() and ziplist() for better traceback message

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH 1 of 4] pycompat: name maplist() and ziplist() for better traceback message

Yuya Nishihara
# HG changeset patch
# User Yuya Nishihara <[hidden email]>
# Date 1520943734 -32400
#      Tue Mar 13 21:22:14 2018 +0900
# Node ID aa35c6d7caef9ef9f48d0c9c831f9492c1421c45
# Parent  6aeb076b1321c540db6419fc48c8a3a552fb596b
pycompat: name maplist() and ziplist() for better traceback message

diff --git a/mercurial/pycompat.py b/mercurial/pycompat.py
--- a/mercurial/pycompat.py
+++ b/mercurial/pycompat.py
@@ -65,8 +65,13 @@ if ispy3:
     if sysexecutable:
         sysexecutable = os.fsencode(sysexecutable)
     stringio = io.BytesIO
-    maplist = lambda *args: list(map(*args))
-    ziplist = lambda *args: list(zip(*args))
+
+    def maplist(*args):
+        return list(map(*args))
+
+    def ziplist(*args):
+        return list(zip(*args))
+
     rawinput = input
     getargspec = inspect.getfullargspec
 
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 2 of 4] annotate: correct parameter name of decorate() function

Yuya Nishihara
# HG changeset patch
# User Yuya Nishihara <[hidden email]>
# Date 1520854628 -32400
#      Mon Mar 12 20:37:08 2018 +0900
# Node ID 95af54712a25c298d5ee63c385f3d498250c1624
# Parent  aa35c6d7caef9ef9f48d0c9c831f9492c1421c45
annotate: correct parameter name of decorate() function

diff --git a/mercurial/dagop.py b/mercurial/dagop.py
--- a/mercurial/dagop.py
+++ b/mercurial/dagop.py
@@ -442,12 +442,12 @@ def annotate(base, parents, linenumber=F
     """
 
     if linenumber:
-        def decorate(text, rev):
-            return ([annotateline(fctx=rev, lineno=i)
+        def decorate(text, fctx):
+            return ([annotateline(fctx=fctx, lineno=i)
                      for i in xrange(1, _countlines(text) + 1)], text)
     else:
-        def decorate(text, rev):
-            return ([annotateline(fctx=rev)] * _countlines(text), text)
+        def decorate(text, fctx):
+            return ([annotateline(fctx=fctx)] * _countlines(text), text)
 
     # This algorithm would prefer to be recursive, but Python is a
     # bit recursion-hostile. Instead we do an iterative
diff --git a/tests/test-annotate.py b/tests/test-annotate.py
--- a/tests/test-annotate.py
+++ b/tests/test-annotate.py
@@ -25,8 +25,8 @@ class AnnotateTests(unittest.TestCase):
         childdata = b'a\nb2\nc\nc2\nd\n'
         diffopts = mdiff.diffopts()
 
-        def decorate(text, rev):
-            return ([annotateline(fctx=rev, lineno=i)
+        def decorate(text, fctx):
+            return ([annotateline(fctx=fctx, lineno=i)
                      for i in range(1, text.count(b'\n') + 1)],
                     text)
 
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 3 of 4] annotate: do not construct attr.s object per line while computing history

Yuya Nishihara
In reply to this post by Yuya Nishihara
# HG changeset patch
# User Yuya Nishihara <[hidden email]>
# Date 1520855110 -32400
#      Mon Mar 12 20:45:10 2018 +0900
# Node ID 1ce0d903cfa6e8db0112f86cc346751651db1f80
# Parent  95af54712a25c298d5ee63c385f3d498250c1624
annotate: do not construct attr.s object per line while computing history

Unfortunately, it's way slower to construct an annotateline() object than
creating a plain tuple or a list. This patch changes the internal data
structure from row-based to columnar, so the decorate() function can be
instant.

  (original)
  $ hg annot mercurial/commands.py --time > /dev/null
  time: real 11.470 secs (user 11.400+0.000 sys 0.070+0.000)
  $ hg annot mercurial/commands.py --time --line-number > /dev/null
  time: real 39.590 secs (user 39.500+0.000 sys 0.080+0.000)

  (this patch)
  $ hg annot mercurial/commands.py --time > /dev/null
  time: real 11.780 secs (user 11.710+0.000 sys 0.070+0.000)
  $ hg annot mercurial/commands.py --time --line-number > /dev/null
  time: real 12.240 secs (user 12.170+0.000 sys 0.090+0.000)

diff --git a/mercurial/dagop.py b/mercurial/dagop.py
--- a/mercurial/dagop.py
+++ b/mercurial/dagop.py
@@ -365,10 +365,19 @@ def blockdescendants(fctx, fromline, tol
 @attr.s(slots=True, frozen=True)
 class annotateline(object):
     fctx = attr.ib()
-    lineno = attr.ib(default=False)
+    lineno = attr.ib()
     # Whether this annotation was the result of a skip-annotate.
     skip = attr.ib(default=False)
 
+@attr.s(slots=True, frozen=True)
+class _annotatedfile(object):
+    # list indexed by lineno - 1
+    fctxs = attr.ib()
+    linenos = attr.ib()
+    skips = attr.ib()
+    # full file content
+    text = attr.ib()
+
 def _countlines(text):
     if text.endswith("\n"):
         return text.count("\n")
@@ -385,7 +394,7 @@ def _annotatepair(parents, childfctx, ch
 
     See test-annotate.py for unit tests.
     '''
-    pblocks = [(parent, mdiff.allblocks(parent[1], child[1], opts=diffopts))
+    pblocks = [(parent, mdiff.allblocks(parent.text, child.text, opts=diffopts))
                for parent in parents]
 
     if skipchild:
@@ -398,7 +407,9 @@ def _annotatepair(parents, childfctx, ch
             # Changed blocks ('!') or blocks made only of blank lines ('~')
             # belong to the child.
             if t == '=':
-                child[0][b1:b2] = parent[0][a1:a2]
+                child.fctxs[b1:b2] = parent.fctxs[a1:a2]
+                child.linenos[b1:b2] = parent.linenos[a1:a2]
+                child.skips[b1:b2] = parent.skips[a1:a2]
 
     if skipchild:
         # Now try and match up anything that couldn't be matched,
@@ -419,9 +430,11 @@ def _annotatepair(parents, childfctx, ch
             for (a1, a2, b1, b2), _t in blocks:
                 if a2 - a1 >= b2 - b1:
                     for bk in xrange(b1, b2):
-                        if child[0][bk].fctx == childfctx:
+                        if child.fctxs[bk] == childfctx:
                             ak = min(a1 + (bk - b1), a2 - 1)
-                            child[0][bk] = attr.evolve(parent[0][ak], skip=True)
+                            child.fctxs[bk] = parent.fctxs[ak]
+                            child.linenos[bk] = parent.linenos[ak]
+                            child.skips[bk] = True
                 else:
                     remaining[idx][1].append((a1, a2, b1, b2))
 
@@ -430,9 +443,11 @@ def _annotatepair(parents, childfctx, ch
         for parent, blocks in remaining:
             for a1, a2, b1, b2 in blocks:
                 for bk in xrange(b1, b2):
-                    if child[0][bk].fctx == childfctx:
+                    if child.fctxs[bk] == childfctx:
                         ak = min(a1 + (bk - b1), a2 - 1)
-                        child[0][bk] = attr.evolve(parent[0][ak], skip=True)
+                        child.fctxs[bk] = parent.fctxs[ak]
+                        child.linenos[bk] = parent.linenos[ak]
+                        child.skips[bk] = True
     return child
 
 def annotate(base, parents, linenumber=False, skiprevs=None, diffopts=None):
@@ -443,11 +458,13 @@ def annotate(base, parents, linenumber=F
 
     if linenumber:
         def decorate(text, fctx):
-            return ([annotateline(fctx=fctx, lineno=i)
-                     for i in xrange(1, _countlines(text) + 1)], text)
+            n = _countlines(text)
+            linenos = pycompat.rangelist(1, n + 1)
+            return _annotatedfile([fctx] * n, linenos, [False] * n, text)
     else:
         def decorate(text, fctx):
-            return ([annotateline(fctx=fctx)] * _countlines(text), text)
+            n = _countlines(text)
+            return _annotatedfile([fctx] * n, [False] * n, [False] * n, text)
 
     # This algorithm would prefer to be recursive, but Python is a
     # bit recursion-hostile. Instead we do an iterative
@@ -501,8 +518,10 @@ def annotate(base, parents, linenumber=F
             hist[f] = curr
             del pcache[f]
 
-    lineattrs, text = hist[base]
-    return pycompat.ziplist(lineattrs, mdiff.splitnewlines(text))
+    a = hist[base]
+    return [(annotateline(fctx, lineno, skip), line)
+            for fctx, lineno, skip, line
+            in zip(a.fctxs, a.linenos, a.skips, mdiff.splitnewlines(a.text))]
 
 def toposort(revs, parentsfunc, firstbranch=()):
     """Yield revisions from heads to roots one (topo) branch at a time.
diff --git a/mercurial/pycompat.py b/mercurial/pycompat.py
--- a/mercurial/pycompat.py
+++ b/mercurial/pycompat.py
@@ -69,6 +69,9 @@ if ispy3:
     def maplist(*args):
         return list(map(*args))
 
+    def rangelist(*args):
+        return list(range(*args))
+
     def ziplist(*args):
         return list(zip(*args))
 
@@ -345,6 +348,7 @@ else:
     shlexsplit = shlex.split
     stringio = cStringIO.StringIO
     maplist = map
+    rangelist = range
     ziplist = zip
     rawinput = raw_input
     getargspec = inspect.getargspec
diff --git a/tests/test-annotate.py b/tests/test-annotate.py
--- a/tests/test-annotate.py
+++ b/tests/test-annotate.py
@@ -5,12 +5,18 @@ import unittest
 
 from mercurial import (
     mdiff,
+    pycompat,
 )
 from mercurial.dagop import (
     annotateline,
+    _annotatedfile,
     _annotatepair,
 )
 
+def tr(a):
+    return [annotateline(fctx, lineno, skip)
+            for fctx, lineno, skip in zip(a.fctxs, a.linenos, a.skips)]
+
 class AnnotateTests(unittest.TestCase):
     """Unit tests for annotate code."""
 
@@ -26,16 +32,16 @@ class AnnotateTests(unittest.TestCase):
         diffopts = mdiff.diffopts()
 
         def decorate(text, fctx):
-            return ([annotateline(fctx=fctx, lineno=i)
-                     for i in range(1, text.count(b'\n') + 1)],
-                    text)
+            n = text.count(b'\n')
+            linenos = pycompat.rangelist(1, n + 1)
+            return _annotatedfile([fctx] * n, linenos, [False] * n, text)
 
         # Basic usage
 
         oldann = decorate(olddata, oldfctx)
         p1ann = decorate(p1data, p1fctx)
         p1ann = _annotatepair([oldann], p1fctx, p1ann, False, diffopts)
-        self.assertEqual(p1ann[0], [
+        self.assertEqual(tr(p1ann), [
             annotateline(b'old', 1),
             annotateline(b'old', 2),
             annotateline(b'p1', 3),
@@ -43,7 +49,7 @@ class AnnotateTests(unittest.TestCase):
 
         p2ann = decorate(p2data, p2fctx)
         p2ann = _annotatepair([oldann], p2fctx, p2ann, False, diffopts)
-        self.assertEqual(p2ann[0], [
+        self.assertEqual(tr(p2ann), [
             annotateline(b'old', 1),
             annotateline(b'p2', 2),
             annotateline(b'p2', 3),
@@ -54,7 +60,7 @@ class AnnotateTests(unittest.TestCase):
         childann = decorate(childdata, childfctx)
         childann = _annotatepair([p1ann, p2ann], childfctx, childann, False,
                                  diffopts)
-        self.assertEqual(childann[0], [
+        self.assertEqual(tr(childann), [
             annotateline(b'old', 1),
             annotateline(b'c', 2),
             annotateline(b'p2', 2),
@@ -65,7 +71,7 @@ class AnnotateTests(unittest.TestCase):
         childann = decorate(childdata, childfctx)
         childann = _annotatepair([p2ann, p1ann], childfctx, childann, False,
                                  diffopts)
-        self.assertEqual(childann[0], [
+        self.assertEqual(tr(childann), [
             annotateline(b'old', 1),
             annotateline(b'c', 2),
             annotateline(b'p1', 3),
@@ -78,7 +84,7 @@ class AnnotateTests(unittest.TestCase):
         childann = decorate(childdata, childfctx)
         childann = _annotatepair([p1ann, p2ann], childfctx, childann, True,
                                  diffopts)
-        self.assertEqual(childann[0], [
+        self.assertEqual(tr(childann), [
             annotateline(b'old', 1),
             annotateline(b'old', 2, True),
             # note that this line was carried over from earlier so it is *not*
@@ -91,7 +97,7 @@ class AnnotateTests(unittest.TestCase):
         childann = decorate(childdata, childfctx)
         childann = _annotatepair([p2ann, p1ann], childfctx, childann, True,
                                  diffopts)
-        self.assertEqual(childann[0], [
+        self.assertEqual(tr(childann), [
             annotateline(b'old', 1),
             annotateline(b'old', 2, True),
             annotateline(b'p1', 3),
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

[PATCH 4 of 4] annotate: drop linenumber flag from fctx.annotate() (API)

Yuya Nishihara
In reply to this post by Yuya Nishihara
# HG changeset patch
# User Yuya Nishihara <[hidden email]>
# Date 1520947086 -32400
#      Tue Mar 13 22:18:06 2018 +0900
# Node ID 854d59569873317764fb7ff68b7f0051585caed6
# Parent  1ce0d903cfa6e8db0112f86cc346751651db1f80
annotate: drop linenumber flag from fctx.annotate() (API)

Now linenumber=True is fast enough to be enabled by default.

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -387,8 +387,8 @@ def annotate(ui, repo, *pats, **opts):
             continue
 
         fm = rootfm.nested('lines')
-        lines = fctx.annotate(follow=follow, linenumber=linenumber,
-                              skiprevs=skiprevs, diffopts=diffopts)
+        lines = fctx.annotate(follow=follow, skiprevs=skiprevs,
+                              diffopts=diffopts)
         if not lines:
             fm.end()
             continue
diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -967,14 +967,13 @@ class basefilectx(object):
             return p[1]
         return filectx(self._repo, self._path, fileid=-1, filelog=self._filelog)
 
-    def annotate(self, follow=False, linenumber=False, skiprevs=None,
-                 diffopts=None):
-        '''returns a list of tuples of ((ctx, number), line) for each line
-        in the file, where ctx is the filectx of the node where
-        that line was last changed; if linenumber parameter is true, number is
-        the line number at the first appearance in the managed file, otherwise,
-        number has a fixed value of False.
-        '''
+    def annotate(self, follow=False, skiprevs=None, diffopts=None):
+        """Returns a list of tuples of (attr, line) for each line in the file
+
+        - attr.fctx is the filectx of the node where that line was last changed
+        - attr.lineno is the line number at the first appearance in the managed
+          file
+        """
         getlog = util.lrucachefunc(lambda x: self._repo.file(x))
 
         def parents(f):
@@ -1010,8 +1009,8 @@ class basefilectx(object):
                 ac = cl.ancestors([base.rev()], inclusive=True)
             base._ancestrycontext = ac
 
-        return dagop.annotate(base, parents, linenumber=linenumber,
-                              skiprevs=skiprevs, diffopts=diffopts)
+        return dagop.annotate(base, parents, skiprevs=skiprevs,
+                              diffopts=diffopts)
 
     def ancestors(self, followfirst=False):
         visit = {}
diff --git a/mercurial/dagop.py b/mercurial/dagop.py
--- a/mercurial/dagop.py
+++ b/mercurial/dagop.py
@@ -383,6 +383,11 @@ def _countlines(text):
         return text.count("\n")
     return text.count("\n") + int(bool(text))
 
+def _decoratelines(text, fctx):
+    n = _countlines(text)
+    linenos = pycompat.rangelist(1, n + 1)
+    return _annotatedfile([fctx] * n, linenos, [False] * n, text)
+
 def _annotatepair(parents, childfctx, child, skipchild, diffopts):
     r'''
     Given parent and child fctxes and annotate data for parents, for all lines
@@ -450,22 +455,12 @@ def _annotatepair(parents, childfctx, ch
                         child.skips[bk] = True
     return child
 
-def annotate(base, parents, linenumber=False, skiprevs=None, diffopts=None):
+def annotate(base, parents, skiprevs=None, diffopts=None):
     """Core algorithm for filectx.annotate()
 
     `parents(fctx)` is a function returning a list of parent filectxs.
     """
 
-    if linenumber:
-        def decorate(text, fctx):
-            n = _countlines(text)
-            linenos = pycompat.rangelist(1, n + 1)
-            return _annotatedfile([fctx] * n, linenos, [False] * n, text)
-    else:
-        def decorate(text, fctx):
-            n = _countlines(text)
-            return _annotatedfile([fctx] * n, [False] * n, [False] * n, text)
-
     # This algorithm would prefer to be recursive, but Python is a
     # bit recursion-hostile. Instead we do an iterative
     # depth-first search.
@@ -502,7 +497,7 @@ def annotate(base, parents, linenumber=F
                 visit.append(p)
         if ready:
             visit.pop()
-            curr = decorate(f.data(), f)
+            curr = _decoratelines(f.data(), f)
             skipchild = False
             if skiprevs is not None:
                 skipchild = f._changeid in skiprevs
diff --git a/mercurial/hgweb/webutil.py b/mercurial/hgweb/webutil.py
--- a/mercurial/hgweb/webutil.py
+++ b/mercurial/hgweb/webutil.py
@@ -186,7 +186,7 @@ def difffeatureopts(req, ui, section):
 
 def annotate(req, fctx, ui):
     diffopts = difffeatureopts(req, ui, 'annotate')
-    return fctx.annotate(follow=True, linenumber=True, diffopts=diffopts)
+    return fctx.annotate(follow=True, diffopts=diffopts)
 
 def parents(ctx, hide=None):
     if isinstance(ctx, context.basefilectx):
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH 1 of 4] pycompat: name maplist() and ziplist() for better traceback message

Pulkit Goyal
In reply to this post by Yuya Nishihara
On Tue, Mar 13, 2018 at 7:25 PM, Yuya Nishihara <[hidden email]> wrote:
> # HG changeset patch
> # User Yuya Nishihara <[hidden email]>
> # Date 1520943734 -32400
> #      Tue Mar 13 21:22:14 2018 +0900
> # Node ID aa35c6d7caef9ef9f48d0c9c831f9492c1421c45
> # Parent  6aeb076b1321c540db6419fc48c8a3a552fb596b
> pycompat: name maplist() and ziplist() for better traceback message

Queued the first two patches. Many thanks!
_______________________________________________
Mercurial-devel mailing list
[hidden email]
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel