Patchwork [2,of,2] profiling: allow loading profiling extension before everything else

login
register
mail settings
Submitter Jun Wu
Date May 22, 2017, 8:24 a.m.
Message ID <f834239fa73503253021.1495441485@x1c>
Download mbox | patch
Permalink /patch/20822/
State Accepted
Headers show

Comments

Jun Wu - May 22, 2017, 8:24 a.m.
# HG changeset patch
# User Jun Wu <quark@fb.com>
# Date 1495441069 25200
#      Mon May 22 01:17:49 2017 -0700
# Node ID f834239fa73503253021149e10c184635f1f20d8
# Parent  fe8b1338edacf98cb13c2be6bdce2823836f1e44
# Available At https://bitbucket.org/quark-zju/hg-draft
#              hg pull https://bitbucket.org/quark-zju/hg-draft -r f834239fa735
profiling: allow loading profiling extension before everything else

6d642ecf1a89 makes profiler start early without loading extensions. That
makes it impossible for an extension to add customized profilers.

This patch adds a special case: if a profiler is not found but an extension
with the same name could be loaded, load that extension first, and expect it
to have a "profile" contextmanager method. This allows customized profilers
and extension setup time is still profiled.
Augie Fackler - May 22, 2017, 7:35 p.m.
On Mon, May 22, 2017 at 01:24:45AM -0700, Jun Wu wrote:
> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1495441069 25200
> #      Mon May 22 01:17:49 2017 -0700
> # Node ID f834239fa73503253021149e10c184635f1f20d8
> # Parent  fe8b1338edacf98cb13c2be6bdce2823836f1e44
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r f834239fa735
> profiling: allow loading profiling extension before everything else

queued, thanks
Gregory Szorc - May 24, 2017, 3:57 p.m.
On Mon, May 22, 2017 at 1:24 AM, Jun Wu <quark@fb.com> wrote:

> # HG changeset patch
> # User Jun Wu <quark@fb.com>
> # Date 1495441069 25200
> #      Mon May 22 01:17:49 2017 -0700
> # Node ID f834239fa73503253021149e10c184635f1f20d8
> # Parent  fe8b1338edacf98cb13c2be6bdce2823836f1e44
> # Available At https://bitbucket.org/quark-zju/hg-draft
> #              hg pull https://bitbucket.org/quark-zju/hg-draft -r
> f834239fa735
> profiling: allow loading profiling extension before everything else
>
> 6d642ecf1a89 makes profiler start early without loading extensions. That
> makes it impossible for an extension to add customized profilers.
>
> This patch adds a special case: if a profiler is not found but an extension
> with the same name could be loaded, load that extension first, and expect
> it
> to have a "profile" contextmanager method. This allows customized profilers
> and extension setup time is still profiled.
>

Am I correct in assuming that Facebook is doing some custom profiling work
again? If so, can it just live upstream?


>
> diff --git a/mercurial/extensions.py b/mercurial/extensions.py
> --- a/mercurial/extensions.py
> +++ b/mercurial/extensions.py
> @@ -182,5 +182,5 @@ def _runextsetup(name, ui):
>  def loadall(ui, whitelist=None):
>      result = ui.configitems("extensions")
> -    if whitelist:
> +    if whitelist is not None:
>          result = [(k, v) for (k, v) in result if k in whitelist]
>      newindex = len(_order)
> diff --git a/mercurial/profiling.py b/mercurial/profiling.py
> --- a/mercurial/profiling.py
> +++ b/mercurial/profiling.py
> @@ -14,7 +14,19 @@ from . import (
>      encoding,
>      error,
> +    extensions,
>      util,
>  )
>
> +def _loadprofiler(ui, profiler):
> +    """load profiler extension. return profile method, or None on
> failure"""
> +    extname = profiler
> +    extensions.loadall(ui, whitelist=[extname])
> +    try:
> +        mod = extensions.find(extname)
> +    except KeyError:
> +        return None
> +    else:
> +        return getattr(mod, 'profile', None)
> +
>  @contextlib.contextmanager
>  def lsprofile(ui, fp):
> @@ -138,9 +150,13 @@ def profile(ui):
>      """
>      profiler = encoding.environ.get('HGPROF')
> +    proffn = None
>      if profiler is None:
>          profiler = ui.config('profiling', 'type', default='stat')
>      if profiler not in ('ls', 'stat', 'flame'):
> -        ui.warn(_("unrecognized profiler '%s' - ignored\n") % profiler)
> -        profiler = 'stat'
> +        # try load profiler from extension with the same name
> +        proffn = _loadprofiler(ui, profiler)
> +        if proffn is None:
> +            ui.warn(_("unrecognized profiler '%s' - ignored\n") %
> profiler)
> +            profiler = 'stat'
>
>      output = ui.config('profiling', 'output')
> @@ -155,5 +171,7 @@ def profile(ui):
>
>      try:
> -        if profiler == 'ls':
> +        if proffn is not None:
> +            pass
> +        elif profiler == 'ls':
>              proffn = lsprofile
>          elif profiler == 'flame':
> diff --git a/tests/test-profile.t b/tests/test-profile.t
> --- a/tests/test-profile.t
> +++ b/tests/test-profile.t
> @@ -100,2 +100,50 @@ statprof can be used as a standalone mod
>
>    $ cd ..
> +
> +profiler extension could be loaded before other extensions
> +
> +  $ cat > fooprof.py <<EOF
> +  > from __future__ import absolute_import
> +  > import contextlib
> +  > @contextlib.contextmanager
> +  > def profile(ui, fp):
> +  >     print('fooprof: start profile')
> +  >     yield
> +  >     print('fooprof: end profile')
> +  > def extsetup(ui):
> +  >     ui.write('fooprof: loaded\n')
> +  > EOF
> +
> +  $ cat > otherextension.py <<EOF
> +  > from __future__ import absolute_import
> +  > def extsetup(ui):
> +  >     ui.write('otherextension: loaded\n')
> +  > EOF
> +
> +  $ hg init b
> +  $ cd b
> +  $ cat >> .hg/hgrc <<EOF
> +  > [extensions]
> +  > other = $TESTTMP/otherextension.py
> +  > fooprof = $TESTTMP/fooprof.py
> +  > EOF
> +
> +  $ hg root
> +  otherextension: loaded
> +  fooprof: loaded
> +  $TESTTMP/b (glob)
> +  $ HGPROF=fooprof hg root --profile
> +  fooprof: loaded
> +  fooprof: start profile
> +  otherextension: loaded
> +  $TESTTMP/b (glob)
> +  fooprof: end profile
> +
> +  $ HGPROF=other hg root --profile 2>&1 | head -n 2
> +  otherextension: loaded
> +  unrecognized profiler 'other' - ignored
> +
> +  $ HGPROF=unknown hg root --profile 2>&1 | head -n 1
> +  unrecognized profiler 'unknown' - ignored
> +
> +  $ cd ..
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Jun Wu - May 24, 2017, 5:33 p.m.
Excerpts from Gregory Szorc's message of 2017-05-24 08:57:28 -0700:
> Am I correct in assuming that Facebook is doing some custom profiling work
> again? If so, can it just live upstream?

This is mostly a side effect of me writing the radixlink index stuff.

Code is at [1] (with some followups). I think upstream wants C code, so C++
and Cython are blockers. I'm currently more focused in lfs, chg, index at
present so cannot prioritize porting the profiler for now.

[1]: https://bitbucket.org/facebook/hg-experimental/commits/d02bde5c791dfc18cf346eca4e163dc09347342d

Patch

diff --git a/mercurial/extensions.py b/mercurial/extensions.py
--- a/mercurial/extensions.py
+++ b/mercurial/extensions.py
@@ -182,5 +182,5 @@  def _runextsetup(name, ui):
 def loadall(ui, whitelist=None):
     result = ui.configitems("extensions")
-    if whitelist:
+    if whitelist is not None:
         result = [(k, v) for (k, v) in result if k in whitelist]
     newindex = len(_order)
diff --git a/mercurial/profiling.py b/mercurial/profiling.py
--- a/mercurial/profiling.py
+++ b/mercurial/profiling.py
@@ -14,7 +14,19 @@  from . import (
     encoding,
     error,
+    extensions,
     util,
 )
 
+def _loadprofiler(ui, profiler):
+    """load profiler extension. return profile method, or None on failure"""
+    extname = profiler
+    extensions.loadall(ui, whitelist=[extname])
+    try:
+        mod = extensions.find(extname)
+    except KeyError:
+        return None
+    else:
+        return getattr(mod, 'profile', None)
+
 @contextlib.contextmanager
 def lsprofile(ui, fp):
@@ -138,9 +150,13 @@  def profile(ui):
     """
     profiler = encoding.environ.get('HGPROF')
+    proffn = None
     if profiler is None:
         profiler = ui.config('profiling', 'type', default='stat')
     if profiler not in ('ls', 'stat', 'flame'):
-        ui.warn(_("unrecognized profiler '%s' - ignored\n") % profiler)
-        profiler = 'stat'
+        # try load profiler from extension with the same name
+        proffn = _loadprofiler(ui, profiler)
+        if proffn is None:
+            ui.warn(_("unrecognized profiler '%s' - ignored\n") % profiler)
+            profiler = 'stat'
 
     output = ui.config('profiling', 'output')
@@ -155,5 +171,7 @@  def profile(ui):
 
     try:
-        if profiler == 'ls':
+        if proffn is not None:
+            pass
+        elif profiler == 'ls':
             proffn = lsprofile
         elif profiler == 'flame':
diff --git a/tests/test-profile.t b/tests/test-profile.t
--- a/tests/test-profile.t
+++ b/tests/test-profile.t
@@ -100,2 +100,50 @@  statprof can be used as a standalone mod
 
   $ cd ..
+
+profiler extension could be loaded before other extensions
+
+  $ cat > fooprof.py <<EOF
+  > from __future__ import absolute_import
+  > import contextlib
+  > @contextlib.contextmanager
+  > def profile(ui, fp):
+  >     print('fooprof: start profile')
+  >     yield
+  >     print('fooprof: end profile')
+  > def extsetup(ui):
+  >     ui.write('fooprof: loaded\n')
+  > EOF
+
+  $ cat > otherextension.py <<EOF
+  > from __future__ import absolute_import
+  > def extsetup(ui):
+  >     ui.write('otherextension: loaded\n')
+  > EOF
+
+  $ hg init b
+  $ cd b
+  $ cat >> .hg/hgrc <<EOF
+  > [extensions]
+  > other = $TESTTMP/otherextension.py
+  > fooprof = $TESTTMP/fooprof.py
+  > EOF
+
+  $ hg root
+  otherextension: loaded
+  fooprof: loaded
+  $TESTTMP/b (glob)
+  $ HGPROF=fooprof hg root --profile
+  fooprof: loaded
+  fooprof: start profile
+  otherextension: loaded
+  $TESTTMP/b (glob)
+  fooprof: end profile
+
+  $ HGPROF=other hg root --profile 2>&1 | head -n 2
+  otherextension: loaded
+  unrecognized profiler 'other' - ignored
+
+  $ HGPROF=unknown hg root --profile 2>&1 | head -n 1
+  unrecognized profiler 'unknown' - ignored
+
+  $ cd ..