changeset 31721:be8a866a2c44

check-code: detect r.revision(r.node(rev)) revlog.revision takes either node or rev, but taking a rev is more efficient, because converting rev to node is just a seek and read. That's cheaper than converting node to rev, which may require O(n) walk in revlog index for the first times, and then triggering building the radix tree index. Even with the radix tree built, rev -> node is still faster than node -> rev because the radix tree requires more jumps in memory. So r.revision(r.node(rev)) should be changed to r.revision(rev). This patch adds a check-code rule to detect that.
author Jun Wu <quark@fb.com>
date Wed, 29 Mar 2017 16:46:57 -0700
parents dea2a17cbfd0
children 4eb75c86368b
files contrib/check-code.py tests/test-check-code.t tests/test-contrib-perf.t
diffstat 3 files changed, 30 insertions(+), 0 deletions(-) [+]
line wrap: on
line diff
--- a/contrib/check-code.py	Wed Mar 29 12:37:03 2017 -0700
+++ b/contrib/check-code.py	Wed Mar 29 16:46:57 2017 -0700
@@ -337,6 +337,8 @@
     (r'^import httplib', "don't use httplib, use util.httplib"),
     (r'^import BaseHTTPServer', "use util.httpserver instead"),
     (r'\.next\(\)', "don't use .next(), use next(...)"),
+    (r'([a-z]*).revision\(\1\.node\(',
+     "don't covert rev to node before passing to revision(nodeorrev)"),
 
     # rules depending on implementation of repquote()
     (r' x+[xpqo%APM][\'"]\n\s+[\'"]x',
--- a/tests/test-check-code.t	Wed Mar 29 12:37:03 2017 -0700
+++ b/tests/test-check-code.t	Wed Mar 29 16:46:57 2017 -0700
@@ -9,7 +9,16 @@
 
   $ hg locate -X contrib/python-zstandard -X hgext/fsmonitor/pywatchman |
   > sed 's-\\-/-g' | xargs "$check_code" --warnings --per-file=0 || false
+  contrib/perf.py:859:
+   >             r.revision(r.node(x))
+   don't covert rev to node before passing to revision(nodeorrev)
   Skipping i18n/polib.py it has no-che?k-code (glob)
+  mercurial/bundlerepo.py:117:
+   >         return mdiff.textdiff(self.revision(self.node(rev1)),
+   don't covert rev to node before passing to revision(nodeorrev)
+  mercurial/bundlerepo.py:118:
+   >                               self.revision(self.node(rev2)))
+   don't covert rev to node before passing to revision(nodeorrev)
   mercurial/demandimport.py:312:
    >     if os.environ.get('HGDEMANDIMPORT') != 'disable':
    use encoding.environ instead (py3)
@@ -36,7 +45,22 @@
   mercurial/policy.py:49:
    >     policy = os.environ.get('HGMODULEPOLICY', policy)
    use encoding.environ instead (py3)
+  mercurial/revlog.py:441:
+   >         t = self.revision(self.node(rev))
+   don't covert rev to node before passing to revision(nodeorrev)
+  mercurial/revlog.py:1599:
+   >                 basetext = self.revision(self.node(baserev), _df=fh, raw=raw)
+   don't covert rev to node before passing to revision(nodeorrev)
+  mercurial/revlog.py:1631:
+   >                     ptext = self.revision(self.node(rev), _df=fh)
+   don't covert rev to node before passing to revision(nodeorrev)
   Skipping mercurial/statprof.py it has no-che?k-code (glob)
+  mercurial/unionrepo.py:93:
+   >         return mdiff.textdiff(self.revision(self.node(rev1)),
+   don't covert rev to node before passing to revision(nodeorrev)
+  mercurial/unionrepo.py:94:
+   >                               self.revision(self.node(rev2)))
+   don't covert rev to node before passing to revision(nodeorrev)
   [1]
 
 @commands in debugcommands.py should be in alphabetical order.
--- a/tests/test-contrib-perf.t	Wed Mar 29 12:37:03 2017 -0700
+++ b/tests/test-contrib-perf.t	Wed Mar 29 16:46:57 2017 -0700
@@ -165,3 +165,7 @@
   $ (hg files -r 1.2 glob:mercurial/*.c glob:mercurial/*.py;
   >  hg files -r tip glob:mercurial/*.c glob:mercurial/*.py) |
   > "$TESTDIR"/check-perf-code.py contrib/perf.py
+  contrib/perf.py:859:
+   >             r.revision(r.node(x))
+   don't covert rev to node before passing to revision(nodeorrev)
+  [1]