Patchwork [6,of,6,RFC] perf: define formatter locally if Mercurial is earlier than 2.2

login
register
mail settings
Submitter Katsunori FUJIWARA
Date June 10, 2016, 7:20 p.m.
Message ID <ab257e0c53440b80edd5.1465586420@feefifofum>
Download mbox | patch
Permalink /patch/15464/
State Changes Requested
Headers show

Comments

Katsunori FUJIWARA - June 10, 2016, 7:20 p.m.
# HG changeset patch
# User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
# Date 1465585986 -32400
#      Sat Jun 11 04:13:06 2016 +0900
# Node ID ab257e0c53440b80edd52e312f6ac8859e6c65b5
# Parent  b5487e8e3142fc650b5fa3d0fbe3e8ff4e24da3e
perf: define formatter locally if Mercurial is earlier than 2.2

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

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

This patch defines formatter class locally, and use it instead of the
value returned by ui.formatter(), if Mercurial is earlier than 2.2 at
runtime.

In this case, we don't need to think about -T/--template option for
formatter, because previous patch made -T/--template disabled for
perf.py with Mercurial earlier than 3.2 (or 7a7eed5176a4).
Pierre-Yves David - June 17, 2016, 5:04 p.m.
The code itself seems mostly good to me (I need a second pass) but I'm a
bit confused about why we want to make perf.py backward compatible? Old
version of Mercurial can use the old version of the perf extension,
isn't that enough? I'm curious about your use case here.

On 06/10/2016 09:20 PM, FUJIWARA Katsunori wrote:
> # HG changeset patch
> # User FUJIWARA Katsunori <foozy@lares.dti.ne.jp>
> # Date 1465585986 -32400
> #      Sat Jun 11 04:13:06 2016 +0900
> # Node ID ab257e0c53440b80edd52e312f6ac8859e6c65b5
> # Parent  b5487e8e3142fc650b5fa3d0fbe3e8ff4e24da3e
> perf: define formatter locally if Mercurial is earlier than 2.2
> 
> Before this patch, using ui.formatter() prevents perf.py from
> measuring performance with Mercurial earlier than 2.2 (or
> ae5f92e154d3), because ui.formatter() isn't available in such
> Mercurial, even though there are some code paths for Mercurial earlier
> than 2.2.
> 
> For example, setting "_prereadsize" attribute in perfindex() and
> perfnodelookup() is effective only with hg earlier than 1.8 (or
> 61c9bc3da402).
> 
> This patch defines formatter class locally, and use it instead of the
> value returned by ui.formatter(), if Mercurial is earlier than 2.2 at
> runtime.
> 
> In this case, we don't need to think about -T/--template option for
> formatter, because previous patch made -T/--template disabled for
> perf.py with Mercurial earlier than 3.2 (or 7a7eed5176a4).
> 
> diff --git a/contrib/perf.py b/contrib/perf.py
> --- a/contrib/perf.py
> +++ b/contrib/perf.py
> @@ -94,7 +94,36 @@ def gettimer(ui, opts=None):
>      ui = ui.copy()
>      ui.fout = ui.ferr
>      # get a formatter
> -    fm = ui.formatter('perf', opts)
> +    if util.safehasattr(ui, 'formatter'):
> +        fm = ui.formatter('perf', opts)
> +    else:
> +        # ui.formatter has been available since 2.2 (or ae5f92e154d3)
> +        from mercurial import node
> +        class defaultformatter(object):
> +            """Minimized composition of baseformatter and plainformatter
> +            """
> +            def __init__(self, ui, topic, opts):
> +                self._ui = ui
> +                if ui.debugflag:
> +                    self.hexfunc = node.hex
> +                else:
> +                    self.hexfunc = node.short
> +            def __nonzero__(self):
> +                return False
> +            def startitem(self):
> +                pass
> +            def data(self, **data):
> +                pass
> +            def write(self, fields, deftext, *fielddata, **opts):
> +                self._ui.write(deftext % fielddata, **opts)
> +            def condwrite(self, cond, fields, deftext, *fielddata, **opts):
> +                if cond:
> +                    self._ui.write(deftext % fielddata, **opts)
> +            def plain(self, text, **opts):
> +                self._ui.write(text, **opts)
> +            def end(self):
> +                pass
> +        fm = defaultformatter(ui, 'perf', opts)
>      # stub function, runs code only once instead of in a loop
>      # experimental config: perf.stub
>      if ui.configbool("perf", "stub"):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Matt Mackall - June 17, 2016, 6:59 p.m.
On Fri, 2016-06-17 at 19:04 +0200, Pierre-Yves David wrote:
> The code itself seems mostly good to me (I need a second pass) but I'm a
> bit confused about why we want to make perf.py backward compatible? Old
> version of Mercurial can use the old version of the perf extension,
> isn't that enough? I'm curious about your use case here.

When you add new perf tests, you want to run them against historic hg. I hit
this regularly.

-- 
Mathematics is the supreme nostalgia of our time.
Pierre-Yves David - June 18, 2016, 1:29 a.m.
On 06/17/2016 08:59 PM, Matt Mackall wrote:
> On Fri, 2016-06-17 at 19:04 +0200, Pierre-Yves David wrote:
>> The code itself seems mostly good to me (I need a second pass) but I'm a
>> bit confused about why we want to make perf.py backward compatible? Old
>> version of Mercurial can use the old version of the perf extension,
>> isn't that enough? I'm curious about your use case here.
> 
> When you add new perf tests, you want to run them against historic hg. I hit
> this regularly.

This quest seems a bit doomed to me as perf.py is likely to keep
evolving alongside the code base of Mercurial, breaking BC and using new
feature. Especially now that there is some people working on improving
our performance tracking.

The way I usually handle this by applying a small patch with the new
perf thing when doing the initial comparision (same as I do with
regression tests)

That said, I've nothing against this series if it help a bit, but I
think we should document (near the compatibility hack) the motive better
so that future contributor understant what is going on here.
Katsunori FUJIWARA - June 18, 2016, 4:28 p.m.
At Sat, 18 Jun 2016 03:29:01 +0200,
Pierre-Yves David wrote:
> 
> On 06/17/2016 08:59 PM, Matt Mackall wrote:
> > On Fri, 2016-06-17 at 19:04 +0200, Pierre-Yves David wrote:
> >> The code itself seems mostly good to me (I need a second pass) but I'm a
> >> bit confused about why we want to make perf.py backward compatible? Old
> >> version of Mercurial can use the old version of the perf extension,
> >> isn't that enough? I'm curious about your use case here.
> > 
> > When you add new perf tests, you want to run them against historic hg. I hit
> > this regularly.
> 
> This quest seems a bit doomed to me as perf.py is likely to keep
> evolving alongside the code base of Mercurial, breaking BC and using new
> feature. Especially now that there is some people working on improving
> our performance tracking.
> 
> The way I usually handle this by applying a small patch with the new
> perf thing when doing the initial comparision (same as I do with
> regression tests)
> 
> That said, I've nothing against this series if it help a bit, but I
> think we should document (near the compatibility hack) the motive better
> so that future contributor understant what is going on here.

I think:

  we have to:

  - make perf.py loadable with as old version hg as possible

  - make historical perf tests work with as old version hg as possible

  we don't have to:

  - make newer perf tests work with old version hg
    (of course, it is good, if a newer test works with old hg)

    therefore, even if perf.py itself can be enabled with an old hg,
    it isn't ensured that all perf tests are executable.

As far as I confirm, we can make recent perf.py loadable with
Mercurial 1.1 or so by this series and some additional patches.


> -- 
> Pierre-Yves David
> 

----------------------------------------------------------------------
[FUJIWARA Katsunori]                             foozy@lares.dti.ne.jp
Pierre-Yves David - June 29, 2016, 9:17 a.m.
On 06/18/2016 06:28 PM, FUJIWARA Katsunori wrote:
> At Sat, 18 Jun 2016 03:29:01 +0200,
> Pierre-Yves David wrote:
>>
>> On 06/17/2016 08:59 PM, Matt Mackall wrote:
>>> On Fri, 2016-06-17 at 19:04 +0200, Pierre-Yves David wrote:
>>>> The code itself seems mostly good to me (I need a second pass) but I'm a
>>>> bit confused about why we want to make perf.py backward compatible? Old
>>>> version of Mercurial can use the old version of the perf extension,
>>>> isn't that enough? I'm curious about your use case here.
>>>
>>> When you add new perf tests, you want to run them against historic hg. I hit
>>> this regularly.
>>
>> This quest seems a bit doomed to me as perf.py is likely to keep
>> evolving alongside the code base of Mercurial, breaking BC and using new
>> feature. Especially now that there is some people working on improving
>> our performance tracking.
>>
>> The way I usually handle this by applying a small patch with the new
>> perf thing when doing the initial comparision (same as I do with
>> regression tests)
>>
>> That said, I've nothing against this series if it help a bit, but I
>> think we should document (near the compatibility hack) the motive better
>> so that future contributor understant what is going on here.
> 
> I think:
> 
>   we have to:
> 
>   - make perf.py loadable with as old version hg as possible
> 
>   - make historical perf tests work with as old version hg as possible
> 
>   we don't have to:
> 
>   - make newer perf tests work with old version hg
>     (of course, it is good, if a newer test works with old hg)
> 
>     therefore, even if perf.py itself can be enabled with an old hg,
>     it isn't ensured that all perf tests are executable.
> 
> As far as I confirm, we can make recent perf.py loadable with
> Mercurial 1.1 or so by this series and some additional patches.

(sorry for the delay)

Okay, can you send a V2 of this series with extra comment about the
intend and boundary of the compatibility effort?

Cheers,

Patch

diff --git a/contrib/perf.py b/contrib/perf.py
--- a/contrib/perf.py
+++ b/contrib/perf.py
@@ -94,7 +94,36 @@  def gettimer(ui, opts=None):
     ui = ui.copy()
     ui.fout = ui.ferr
     # get a formatter
-    fm = ui.formatter('perf', opts)
+    if util.safehasattr(ui, 'formatter'):
+        fm = ui.formatter('perf', opts)
+    else:
+        # ui.formatter has been available since 2.2 (or ae5f92e154d3)
+        from mercurial import node
+        class defaultformatter(object):
+            """Minimized composition of baseformatter and plainformatter
+            """
+            def __init__(self, ui, topic, opts):
+                self._ui = ui
+                if ui.debugflag:
+                    self.hexfunc = node.hex
+                else:
+                    self.hexfunc = node.short
+            def __nonzero__(self):
+                return False
+            def startitem(self):
+                pass
+            def data(self, **data):
+                pass
+            def write(self, fields, deftext, *fielddata, **opts):
+                self._ui.write(deftext % fielddata, **opts)
+            def condwrite(self, cond, fields, deftext, *fielddata, **opts):
+                if cond:
+                    self._ui.write(deftext % fielddata, **opts)
+            def plain(self, text, **opts):
+                self._ui.write(text, **opts)
+            def end(self):
+                pass
+        fm = defaultformatter(ui, 'perf', opts)
     # stub function, runs code only once instead of in a loop
     # experimental config: perf.stub
     if ui.configbool("perf", "stub"):