Patchwork [3,of,5] perf: don't convert rev to node before calling revlog.revision()

login
register
mail settings
Submitter Gregory Szorc
Date May 6, 2017, 6:21 p.m.
Message ID <4ba2d6cb2cb76a5c18ed.1494094919@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/20488/
State Accepted
Headers show

Comments

Gregory Szorc - May 6, 2017, 6:21 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1494094343 25200
#      Sat May 06 11:12:23 2017 -0700
# Node ID 4ba2d6cb2cb76a5c18ed20ae3ee0f89f2a50eb72
# Parent  2f0c421e15eb86d4d45ce96d559052c919407afd
perf: don't convert rev to node before calling revlog.revision()
Yuya Nishihara - May 9, 2017, 1:23 p.m.
On Sat, 06 May 2017 11:21:59 -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1494094343 25200
> #      Sat May 06 11:12:23 2017 -0700
> # Node ID 4ba2d6cb2cb76a5c18ed20ae3ee0f89f2a50eb72
> # Parent  2f0c421e15eb86d4d45ce96d559052c919407afd
> perf: don't convert rev to node before calling revlog.revision()
> 
> diff --git a/contrib/perf.py b/contrib/perf.py
> --- a/contrib/perf.py
> +++ b/contrib/perf.py
> @@ -865,7 +865,7 @@ def perfrevlog(ui, repo, file_=None, sta
>              dist = -1 * dist
>  
>          for x in xrange(beginrev, endrev, dist):
> -            r.revision(r.node(x))
> +            r.revision(x)

IIRC, this was kept unchanged for some reason. Jun?
Jun Wu - May 10, 2017, 8:07 a.m.
Excerpts from Yuya Nishihara's message of 2017-05-09 22:23:03 +0900:
> > diff --git a/contrib/perf.py b/contrib/perf.py
> > --- a/contrib/perf.py
> > +++ b/contrib/perf.py
> > @@ -865,7 +865,7 @@ def perfrevlog(ui, repo, file_=None, sta
> >              dist = -1 * dist
> >  
> >          for x in xrange(beginrev, endrev, dist):
> > -            r.revision(r.node(x))
> > +            r.revision(x)
> 
> IIRC, this was kept unchanged for some reason. Jun?

Yes. tl;dr hg earlier than 9117c6561b0b does not support nodeorrev.

See https://patchwork.mercurial-scm.org/patch/19836/
Yuya Nishihara - May 10, 2017, 1:47 p.m.
On Wed, 10 May 2017 01:07:50 -0700, Jun Wu wrote:
> Excerpts from Yuya Nishihara's message of 2017-05-09 22:23:03 +0900:
> > > diff --git a/contrib/perf.py b/contrib/perf.py
> > > --- a/contrib/perf.py
> > > +++ b/contrib/perf.py
> > > @@ -865,7 +865,7 @@ def perfrevlog(ui, repo, file_=None, sta
> > >              dist = -1 * dist
> > >  
> > >          for x in xrange(beginrev, endrev, dist):
> > > -            r.revision(r.node(x))
> > > +            r.revision(x)
> > 
> > IIRC, this was kept unchanged for some reason. Jun?
> 
> Yes. tl;dr hg earlier than 9117c6561b0b does not support nodeorrev.
> 
> See https://patchwork.mercurial-scm.org/patch/19836/

Thanks.

Gregory, can you send a follow-up patch? I don't want to drop the other
patches because of this.
Gregory Szorc - May 13, 2017, 9:23 p.m.
On Wed, May 10, 2017 at 6:47 AM, Yuya Nishihara <yuya@tcha.org> wrote:

> On Wed, 10 May 2017 01:07:50 -0700, Jun Wu wrote:
> > Excerpts from Yuya Nishihara's message of 2017-05-09 22:23:03 +0900:
> > > > diff --git a/contrib/perf.py b/contrib/perf.py
> > > > --- a/contrib/perf.py
> > > > +++ b/contrib/perf.py
> > > > @@ -865,7 +865,7 @@ def perfrevlog(ui, repo, file_=None, sta
> > > >              dist = -1 * dist
> > > >
> > > >          for x in xrange(beginrev, endrev, dist):
> > > > -            r.revision(r.node(x))
> > > > +            r.revision(x)
> > >
> > > IIRC, this was kept unchanged for some reason. Jun?
> >
> > Yes. tl;dr hg earlier than 9117c6561b0b does not support nodeorrev.
> >
> > See https://patchwork.mercurial-scm.org/patch/19836/
>
> Thanks.
>
> Gregory, can you send a follow-up patch? I don't want to drop the other
> patches because of this.
>

It looks like the series got queued as 73c3e226d2fc.

If you drop 73c3e226d2fc::4c6b2076d292 from hg-committed, I'll resend them.
(I figure it is probably too difficult to send patches when the series is
already queued since it is unclear how to apply them.)
Yuya Nishihara - May 14, 2017, 3:16 a.m.
On Sat, 13 May 2017 14:23:23 -0700, Gregory Szorc wrote:
> On Wed, May 10, 2017 at 6:47 AM, Yuya Nishihara <yuya@tcha.org> wrote:
> 
> > On Wed, 10 May 2017 01:07:50 -0700, Jun Wu wrote:
> > > Excerpts from Yuya Nishihara's message of 2017-05-09 22:23:03 +0900:
> > > > > diff --git a/contrib/perf.py b/contrib/perf.py
> > > > > --- a/contrib/perf.py
> > > > > +++ b/contrib/perf.py
> > > > > @@ -865,7 +865,7 @@ def perfrevlog(ui, repo, file_=None, sta
> > > > >              dist = -1 * dist
> > > > >
> > > > >          for x in xrange(beginrev, endrev, dist):
> > > > > -            r.revision(r.node(x))
> > > > > +            r.revision(x)
> > > >
> > > > IIRC, this was kept unchanged for some reason. Jun?
> > >
> > > Yes. tl;dr hg earlier than 9117c6561b0b does not support nodeorrev.
> > >
> > > See https://patchwork.mercurial-scm.org/patch/19836/
> >
> > Thanks.
> >
> > Gregory, can you send a follow-up patch? I don't want to drop the other
> > patches because of this.
> >
> 
> It looks like the series got queued as 73c3e226d2fc.
> 
> If you drop 73c3e226d2fc::4c6b2076d292 from hg-committed, I'll resend them.
> (I figure it is probably too difficult to send patches when the series is
> already queued since it is unclear how to apply them.)

Can you make new patch on top of that? I hesitate to do a large rebase on
patches that's already been accepted.

FWIW, maybe we'll need to introduce a magic comment like '# re-raises' to
silence 'r.revision(r.node())' warning explicitly.

Patch

diff --git a/contrib/perf.py b/contrib/perf.py
--- a/contrib/perf.py
+++ b/contrib/perf.py
@@ -865,7 +865,7 @@  def perfrevlog(ui, repo, file_=None, sta
             dist = -1 * dist
 
         for x in xrange(beginrev, endrev, dist):
-            r.revision(r.node(x))
+            r.revision(x)
 
     timer, fm = gettimer(ui, opts)
     timer(d)
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,15 +9,11 @@  New errors are not allowed. Warnings are
 
   $ hg locate -X contrib/python-zstandard -X hgext/fsmonitor/pywatchman |
   > sed 's-\\-/-g' | "$check_code" --warnings --per-file=0 - || false
-  contrib/perf.py:869:
-   >             r.revision(r.node(x))
-   don't convert rev to node before passing to revision(nodeorrev)
   Skipping i18n/polib.py it has no-che?k-code (glob)
   Skipping mercurial/httpclient/__init__.py it has no-che?k-code (glob)
   Skipping mercurial/httpclient/_readers.py it has no-che?k-code (glob)
   Skipping mercurial/statprof.py it has no-che?k-code (glob)
   Skipping tests/badserverext.py it has no-che?k-code (glob)
-  [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,7 +165,3 @@  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:869:
-   >             r.revision(r.node(x))
-   don't convert rev to node before passing to revision(nodeorrev)
-  [1]