# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 2017-03-29 23:46:57
# Node ID be8a866a2c44184e82a43214837bb0ec40a8ad93
# Parent  dea2a17cbfd00bf08ee87b3e44b1c71499189f89

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.

diff --git a/contrib/check-code.py b/contrib/check-code.py
--- a/contrib/check-code.py
+++ b/contrib/check-code.py
@@ -337,6 +337,8 @@ pypats = [
     (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',
diff --git a/tests/test-check-code.t b/tests/test-check-code.t
--- a/tests/test-check-code.t
+++ b/tests/test-check-code.t
@@ -9,7 +9,16 @@ New errors are not allowed. Warnings are
 
   $ 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 @@ New errors are not allowed. Warnings are
   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.
diff --git a/tests/test-contrib-perf.t b/tests/test-contrib-perf.t
--- a/tests/test-contrib-perf.t
+++ b/tests/test-contrib-perf.t
@@ -165,3 +165,7 @@ Check perf.py for historical portability
   $ (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]