Patchwork [6,of,6,V2] perf: make perf command import newer module locally for earlier Mercurial

login
register
mail settings
Submitter Katsunori FUJIWARA
Date July 4, 2016, 10:37 p.m.
Message ID <92b86498070cd8be6320.1467671827@feefifofum>
Download mbox | patch
Permalink /patch/15750/
State Changes Requested
Headers show

Comments

Katsunori FUJIWARA - July 4, 2016, 10:37 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1467671151 -32400
#      Tue Jul 05 07:25:51 2016 +0900
# Node ID 92b86498070cd8be6320da214203a67c0b257bde
# Parent  c19962592bbce7ffd5e82450d2e6cb40ceed90f9
perf: make perf command import newer module locally for earlier Mercurial

demandimport of early Mercurial loads an imported module immediately,
if a module is imported absolutely by "from a import b" style. Recent
perf.py satisfies this condition, because it does:

  - have "from __future__ import absolute_import" line
  - use "from a import b" style for modules in "mercurial" package

Before this patch, importing modules below prevents perf.py from being
loaded by earlier Mercurial, because these aren't available in such
Mercurial, even though there are some code paths for Mercurial earlier
than 1.9.

  - branchmap 2.5 (or bcee63733aad)
  - repoview  2.5 (or 3a6ddacb7198)
  - obsolete  2.3 (or ad0d6c2b3279)
  - scmutil   1.9 (or 8b252e826c68)

For example, setting "_prereadsize" attribute in perfindex() and
perfnodelookup() is effective only with Mercurial earlier than 1.8 (or
61c9bc3da402).

This patch makes modules introduced after 1.2 loaded locally in each
perf commands requiring them.

This patch also adds check-code rule for perf.py, to detect importing
newer module in the future.

"mercurial.error" module is a blocker for loading perf.py with
Mercurial earlier than 1.2. And just making functions import
"mercurial.error" locally isn't enough for historical
portability. This is reason why this patch uses "1.2" as earlier side
version of "white list" in check-code.
Yuya Nishihara - July 9, 2016, 12:02 p.m.
On Tue, 05 Jul 2016 07:37:07 +0900, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1467671151 -32400
> #      Tue Jul 05 07:25:51 2016 +0900
> # Node ID 92b86498070cd8be6320da214203a67c0b257bde
> # Parent  c19962592bbce7ffd5e82450d2e6cb40ceed90f9
> perf: make perf command import newer module locally for earlier Mercurial

> +# modules, which are available both at 1.2 and 3.8
> +_modwhitelist = '''
> +ancestor
> +archival
> +base85
> +bdiff
> +bundlerepo
> +byterange
> +changegroup
> +changelog
> +cmdutil
> +commands
> +context
> +copies
> +demandimport
> +diffhelpers
> +dirstate
> +dispatch
> +error
> +extensions
> +fancyopts
> +filelog
> +filemerge
> +graphmod
> +hbisect
> +help
> +hg
> +hook
> +i18n
> +keepalive
> +localrepo
> +lock
> +lsprof
> +mail
> +manifest
> +match
> +mdiff
> +merge
> +mpatch
> +node
> +osutil
> +parsers
> +patch
> +repair
> +revlog
> +simplemerge
> +sshserver
> +statichttprepo
> +store
> +streamclone
> +strutil
> +templatefilters
> +templater
> +transaction
> +ui
> +url
> +util
> +verify
> +'''.split()

Can't be a separate test-check-pref.t and generate the list by hg commands?

> diff --git a/contrib/perf.py b/contrib/perf.py
> --- a/contrib/perf.py
> +++ b/contrib/perf.py
> @@ -25,7 +25,6 @@ import random
>  import sys
>  import time
>  from mercurial import (
> -    branchmap,
>      cmdutil,
>      commands,
>      copies,
> @@ -33,10 +32,7 @@ from mercurial import (
>      extensions,
>      mdiff,
>      merge,
> -    obsolete,
> -    repoview,
>      revlog,
> -    scmutil,
>      util,
>  )
>  
> @@ -167,6 +163,11 @@ def _timer(fm, func, title=None):
>  
>  @command('perfwalk', formatteropts)
>  def perfwalk(ui, repo, *pats, **opts):
> +    # for "historical portability":
> +    # import locally, because:
> +    # - scmutil has been available since 1.9 (or 8b252e826c68)
> +    from mercurial import scmutil

I prefer conditional import at top rather than copying portability comment
to every place where scmutil is used. Optionally, @command decorator could
be extended to omit unsupported commands from cmdtable, e.g.

  @command('prefwalk', formatteropts, requiremods='scmutil')

Patch

diff --git a/contrib/check-code.py b/contrib/check-code.py
--- a/contrib/check-code.py
+++ b/contrib/check-code.py
@@ -441,6 +441,76 @@  webtemplatepats = [
   ]
 ]
 
+# modules, which are available both at 1.2 and 3.8
+_modwhitelist = '''
+ancestor
+archival
+base85
+bdiff
+bundlerepo
+byterange
+changegroup
+changelog
+cmdutil
+commands
+context
+copies
+demandimport
+diffhelpers
+dirstate
+dispatch
+error
+extensions
+fancyopts
+filelog
+filemerge
+graphmod
+hbisect
+help
+hg
+hook
+i18n
+keepalive
+localrepo
+lock
+lsprof
+mail
+manifest
+match
+mdiff
+merge
+mpatch
+node
+osutil
+parsers
+patch
+repair
+revlog
+simplemerge
+sshserver
+statichttprepo
+store
+streamclone
+strutil
+templatefilters
+templater
+transaction
+ui
+url
+util
+verify
+'''.split()
+
+perfpypats = [
+  [
+    ((r'from mercurial import [(][a-z0-9, \n#]*\n(?! *%s,|^[ \n#]*$|[)])'
+      % ',| *'.join(_modwhitelist)),
+     "import newer module locally in each perf function for early Mercurial"),
+  ],
+  # warnings
+  []
+]
+
 checks = [
     ('python', r'.*\.(py|cgi)$', r'^#!.*python', pyfilters, pypats),
     ('test script', r'(.*/)?test-[^.~]*$', '', testfilters, testpats),
@@ -453,6 +523,7 @@  checks = [
     ('txt', r'.*\.txt$', '', txtfilters, txtpats),
     ('web template', r'mercurial/templates/.*\.tmpl', '',
      webtemplatefilters, webtemplatepats),
+    ('perf.py', r'contrib/perf.py$', '', pyfilters, perfpypats),
 ]
 
 def _preparepats():
diff --git a/contrib/perf.py b/contrib/perf.py
--- a/contrib/perf.py
+++ b/contrib/perf.py
@@ -25,7 +25,6 @@  import random
 import sys
 import time
 from mercurial import (
-    branchmap,
     cmdutil,
     commands,
     copies,
@@ -33,10 +32,7 @@  from mercurial import (
     extensions,
     mdiff,
     merge,
-    obsolete,
-    repoview,
     revlog,
-    scmutil,
     util,
 )
 
@@ -167,6 +163,11 @@  def _timer(fm, func, title=None):
 
 @command('perfwalk', formatteropts)
 def perfwalk(ui, repo, *pats, **opts):
+    # for "historical portability":
+    # import locally, because:
+    # - scmutil has been available since 1.9 (or 8b252e826c68)
+    from mercurial import scmutil
+
     timer, fm = gettimer(ui, opts)
     try:
         m = scmutil.match(repo[None], pats, {})
@@ -199,6 +200,11 @@  def perfstatus(ui, repo, **opts):
 
 @command('perfaddremove', formatteropts)
 def perfaddremove(ui, repo, **opts):
+    # for "historical portability":
+    # import locally, because:
+    # - scmutil has been available since 1.9 (or 8b252e826c68)
+    from mercurial import scmutil
+
     timer, fm = gettimer(ui, opts)
     try:
         oldquiet = repo.ui.quiet
@@ -331,6 +337,11 @@  def perfdirstatewrite(ui, repo, **opts):
 @command('perfmergecalculate',
          [('r', 'rev', '.', 'rev to merge against')] + formatteropts)
 def perfmergecalculate(ui, repo, rev, **opts):
+    # for "historical portability":
+    # import locally, because:
+    # - scmutil has been available since 1.9 (or 8b252e826c68)
+    from mercurial import scmutil
+
     timer, fm = gettimer(ui, opts)
     wctx = repo[None]
     rctx = scmutil.revsingle(repo, rev, rev)
@@ -348,6 +359,11 @@  def perfmergecalculate(ui, repo, rev, **
 
 @command('perfpathcopies', [], "REV REV")
 def perfpathcopies(ui, repo, rev1, rev2, **opts):
+    # for "historical portability":
+    # import module locally, because:
+    # - scmutil has been available since 1.9 (or 8b252e826c68)
+    from mercurial import scmutil
+
     timer, fm = gettimer(ui, opts)
     ctx1 = scmutil.revsingle(repo, rev1, rev1)
     ctx2 = scmutil.revsingle(repo, rev2, rev2)
@@ -358,6 +374,11 @@  def perfpathcopies(ui, repo, rev1, rev2,
 
 @command('perfmanifest', [], 'REV')
 def perfmanifest(ui, repo, rev, **opts):
+    # for "historical portability":
+    # import locally, because:
+    # - scmutil has been available since 1.9 (or 8b252e826c68)
+    from mercurial import scmutil
+
     timer, fm = gettimer(ui, opts)
     ctx = scmutil.revsingle(repo, rev, rev)
     t = ctx.manifestnode()
@@ -445,6 +466,11 @@  def perflookup(ui, repo, rev, **opts):
 
 @command('perfrevrange', formatteropts)
 def perfrevrange(ui, repo, *specs, **opts):
+    # for "historical portability":
+    # import locally, because:
+    # - scmutil has been available since 1.9 (or 8b252e826c68)
+    from mercurial import scmutil
+
     timer, fm = gettimer(ui, opts)
     revrange = scmutil.revrange
     timer(lambda: len(revrange(repo, specs)))
@@ -503,6 +529,11 @@  def perftemplating(ui, repo, rev=None, *
 
 @command('perfcca', formatteropts)
 def perfcca(ui, repo, **opts):
+    # for "historical portability":
+    # import locally, because:
+    # - scmutil has been available since 1.9 (or 8b252e826c68)
+    from mercurial import scmutil
+
     timer, fm = gettimer(ui, opts)
     timer(lambda: scmutil.casecollisionauditor(ui, False, repo.dirstate))
     fm.end()
@@ -701,6 +732,12 @@  def perfvolatilesets(ui, repo, *names, *
     """benchmark the computation of various volatile set
 
     Volatile set computes element related to filtering and obsolescence."""
+    # for "historical portability":
+    # import locally, because:
+    # - obsolete has been available since 2.3 (or ad0d6c2b3279)
+    # - repoview has been available since 2.5 (or 3a6ddacb7198)
+    from mercurial import obsolete, repoview
+
     timer, fm = gettimer(ui, opts)
     repo = repo.unfiltered()
 
@@ -740,6 +777,12 @@  def perfbranchmap(ui, repo, full=False, 
 
     This benchmarks the full repo.branchmap() call with read and write disabled
     """
+    # for "historical portability":
+    # import locally, because:
+    # - branchmap has been available since 2.5 (or bcee63733aad)
+    # - repoview has been available since 2.5 (or 3a6ddacb7198)
+    from mercurial import branchmap, repoview
+
     timer, fm = gettimer(ui, opts)
     def getbranchmap(filtername):
         """generate a benchmark function for the filtername"""
@@ -790,6 +833,11 @@  def perfloadmarkers(ui, repo):
     """benchmark the time to parse the on-disk markers for a repo
 
     Result is the number of markers in the repo."""
+    # for "historical portability":
+    # import locally, because:
+    # - obsolete has been available since 2.3 (or ad0d6c2b3279)
+    from mercurial import obsolete
+
     timer, fm = gettimer(ui)
     timer(lambda: len(obsolete.obsstore(repo.svfs)))
     fm.end()