Patchwork [8,of,8] perf: replace ui.configint() by getint() for Mercurial earlier than 1.9

login
register
mail settings
Submitter Katsunori FUJIWARA
Date Aug. 8, 2016, 9:59 a.m.
Message ID <1c1edce43be9191c6e4d.1470650387@juju>
Download mbox | patch
Permalink /patch/16196/
State Changes Requested
Delegated to: Yuya Nishihara
Headers show

Comments

Katsunori FUJIWARA - Aug. 8, 2016, 9:59 a.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1470649698 -32400
#      Mon Aug 08 18:48:18 2016 +0900
# Node ID 1c1edce43be9191c6e4d6d91ee02c5299d09a4f7
# Parent  697b11a0c95313d8220f6b5a1f570125c85e3e43
perf: replace ui.configint() by getint() for Mercurial earlier than 1.9

Before this patch, using ui.configint() prevents perf.py from
measuring performance with Mercurial earlier than 1.9 (or
fa2b596db182), because ui.configint() isn't available in such
Mercurial, even though there are some code paths for Mercurial earlier
than 1.9.

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

This patch replaces ui.configint() invocations by newly introduced
getint().

This patch also adds check-perf-code.py an extra check entry to detect
direct usage of ui.configint() in perf.py.

BTW, this patch doesn't choose adding configint() method at runtime by
replacing ui.__class__ like below, even though this is the recommended
way to modern Mercurial extensions.

    def uisetup(ui):
        if not util.safehasattr(ui, 'configint'):
            class uiwrap(ui.__class__):
                def configint(self, section, name, ....):
                    ....
            ui.__class__ = uiwrap

Because changes to ui.__class__ by uisetup() of loaded extension have
been propagated since 1.6.1 (or d8d0fc3988ca), the recommended way
above doesn't work as expected with Mercurial earlier than it.
timeless - Aug. 11, 2016, 6:43 a.m.
FUJIWARA Katsunori wrote:
> +    v  = ui.config(section, name, None)

fwiw, you have two spaces before `=`
Katsunori FUJIWARA - Aug. 12, 2016, 12:50 a.m.
At Thu, 11 Aug 2016 02:43:33 -0400,
timeless wrote:
> 
> FUJIWARA Katsunori wrote:
> > +    v  = ui.config(section, name, None)
> 
> fwiw, you have two spaces before `=`
> 

Thank you for pointing out!

I expect check-code.py to detect this kind of problem, but now I
notice that it checks only gratuitous whitespace "after" =. Maybe, for
some column positioning like as below ?

    https://selenic.com/repo/hg/file/3.9/hgext/convert/monotone.py#l47
    https://selenic.com/repo/hg/file/3.9/mercurial/cmdutil.py#l3031

I'll fix that line in V2.

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
timeless - Aug. 12, 2016, 1:14 a.m.
> I expect check-code.py to detect this kind of problem, but now I
> notice that it checks only gratuitous whitespace "after" =. Maybe, for
> some column positioning like as below ?

Yeah, that's the reason it doesn't.
It's probably possible to try to detect blocks that are trying to
align, but that's really overkill.

Patch

diff --git a/contrib/perf.py b/contrib/perf.py
--- a/contrib/perf.py
+++ b/contrib/perf.py
@@ -130,7 +130,7 @@  def gettimer(ui, opts=None):
 
     # enforce an idle period before execution to counteract power management
     # experimental config: perf.presleep
-    time.sleep(ui.configint("perf", "presleep", 1))
+    time.sleep(getint(ui, "perf", "presleep", 1))
 
     if opts is None:
         opts = {}
@@ -221,6 +221,18 @@  def _timer(fm, func, title=None):
 
 # utilities for historical portability
 
+def getint(ui, section, name, default):
+    # for "historical portability":
+    # ui.configint has been available since 1.9 (or fa2b596db182)
+    v  = ui.config(section, name, None)
+    if v is None:
+        return default
+    try:
+        return int(v)
+    except ValueError:
+        raise error.ConfigError(("%s.%s is not an integer ('%s')")
+                                % (section, name, v))
+
 def safeattrsetter(obj, name, ignoremissing=False):
     """Ensure that 'obj' has 'name' attribute before execution of setattr
 
@@ -568,7 +580,7 @@  def perfparents(ui, repo, **opts):
     timer, fm = gettimer(ui, opts)
     # control the number of commits perfparents iterates over
     # experimental config: perf.parentscount
-    count = ui.configint("perf", "parentscount", 1000)
+    count = getint(ui, "perf", "parentscount", 1000)
     if len(repo.changelog) < count:
         raise error.Abort("repo needs %d commits for this test" % count)
     repo = repo.unfiltered()
diff --git a/tests/check-perf-code.py b/tests/check-perf-code.py
--- a/tests/check-perf-code.py
+++ b/tests/check-perf-code.py
@@ -14,6 +14,8 @@  perfpypats = [
      "use getbranchmapsubsettable() for early Mercurial"),
     (r'\.(vfs|svfs|opener|sopener)',
      "use getvfs()/getsvfs() for early Mercurial"),
+    (r'ui\.configint',
+     "use getint() instead of ui.configint() for early Mercurial"),
   ],
   # warnings
   [