Patchwork [4,of,7,V2] profiling: add a context manager that no-ops if profiling isn't enabled

login
register
mail settings
Submitter Gregory Szorc
Date Aug. 15, 2016, 1:37 a.m.
Message ID <fd7b55561853e9d5532f.1471225061@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/16298/
State Accepted
Headers show

Comments

Gregory Szorc - Aug. 15, 2016, 1:37 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1471222272 25200
#      Sun Aug 14 17:51:12 2016 -0700
# Node ID fd7b55561853e9d5532f3a9265433a7b614f2687
# Parent  3338d4536f490fca055881b4e3884b28e4e25d22
profiling: add a context manager that no-ops if profiling isn't enabled

And refactor dispatch.py to use it. As you can see, the resulting code
is much simpler.

I was tempted to inline _runcommand as part of writing this series.
However, a number of extensions wrap _runcommand. So keeping it around
is necessary (extensions can't easily wrap runcommand because it calls
hooks before and after command execution).
Yuya Nishihara - Aug. 16, 2016, 4:19 a.m.
On Sun, 14 Aug 2016 18:37:41 -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1471222272 25200
> #      Sun Aug 14 17:51:12 2016 -0700
> # Node ID fd7b55561853e9d5532f3a9265433a7b614f2687
> # Parent  3338d4536f490fca055881b4e3884b28e4e25d22
> profiling: add a context manager that no-ops if profiling isn't enabled

> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -1388,16 +1388,22 @@ Specifies profiling type, format, and fi
>  supported: an instrumenting profiler (named ``ls``), and a sampling
>  profiler (named ``stat``).
>  
>  In this section description, 'profiling data' stands for the raw data
>  collected during profiling, while 'profiling report' stands for a
>  statistical text report generated from the profiling data. The
>  profiling is done using lsprof.
>  
> +``enabled``
> +    Enable the profiler.
> +    (default: false)
> +
> +    This is equivalent to passing ``--profile`` on the command line.
> +

> +def maybeprofile(ui):
> +    """Profile if enabled, else do nothing.
> +
> +    This context manager can be used to optionally profile if profiling
> +    is enabled. Otherwise, it does nothing.
> +
> +    The purpose of this context manager is to make calling code simpler:
> +    just use a single code path for calling into code you may want to profile
> +    and this function determines whether to start profiling.
> +    """
> +    # internal config profiling.enabled

Removed this line because profiling.enabled is now documented.

Patch

diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -893,31 +893,22 @@  def _dispatch(req):
     try:
         return runcommand(lui, repo, cmd, fullargs, ui, options, d,
                           cmdpats, cmdoptions)
     finally:
         if repo and repo != req.repo:
             repo.close()
 
 def _runcommand(ui, options, cmd, cmdfunc):
-    """Enables the profiler if applicable.
-
-    ``profiling.enabled`` - boolean config that enables or disables profiling
-    """
-    def checkargs():
+    """Run a command function, possibly with profiling enabled."""
+    with profiling.maybeprofile(ui):
         try:
             return cmdfunc()
         except error.SignatureError:
-            raise error.CommandError(cmd, _("invalid arguments"))
-
-    if ui.configbool('profiling', 'enabled'):
-        with profiling.profile(ui):
-            return checkargs()
-    else:
-        return checkargs()
+            raise error.CommandError(cmd, _('invalid arguments'))
 
 def _exceptionwarning(ui):
     """Produce a warning message for the current active exception"""
 
     # For compatibility checking, we discard the portion of the hg
     # version after the + on the assumption that if a "normal
     # user" is running a build with a + in it the packager
     # probably built from fairly close to a tag and anyone with a
diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
--- a/mercurial/help/config.txt
+++ b/mercurial/help/config.txt
@@ -1388,16 +1388,22 @@  Specifies profiling type, format, and fi
 supported: an instrumenting profiler (named ``ls``), and a sampling
 profiler (named ``stat``).
 
 In this section description, 'profiling data' stands for the raw data
 collected during profiling, while 'profiling report' stands for a
 statistical text report generated from the profiling data. The
 profiling is done using lsprof.
 
+``enabled``
+    Enable the profiler.
+    (default: false)
+
+    This is equivalent to passing ``--profile`` on the command line.
+
 ``type``
     The type of profiler to use.
     (default: ls)
 
     ``ls``
       Use Python's built-in instrumenting profiler. This profiler
       works on all platforms, but each line number it reports is the
       first line of a function. This restriction makes it difficult to
diff --git a/mercurial/profiling.py b/mercurial/profiling.py
--- a/mercurial/profiling.py
+++ b/mercurial/profiling.py
@@ -138,8 +138,26 @@  def profile(ui):
         if output:
             if output == 'blackbox':
                 val = 'Profile:\n%s' % fp.getvalue()
                 # ui.log treats the input as a format string,
                 # so we need to escape any % signs.
                 val = val.replace('%', '%%')
                 ui.log('profile', val)
             fp.close()
+
+@contextlib.contextmanager
+def maybeprofile(ui):
+    """Profile if enabled, else do nothing.
+
+    This context manager can be used to optionally profile if profiling
+    is enabled. Otherwise, it does nothing.
+
+    The purpose of this context manager is to make calling code simpler:
+    just use a single code path for calling into code you may want to profile
+    and this function determines whether to start profiling.
+    """
+    # internal config profiling.enabled
+    if ui.configbool('profiling', 'enabled'):
+        with profile(ui):
+            yield
+    else:
+        yield
diff --git a/tests/test-devel-warnings.t b/tests/test-devel-warnings.t
--- a/tests/test-devel-warnings.t
+++ b/tests/test-devel-warnings.t
@@ -91,17 +91,16 @@ 
    */mercurial/dispatch.py:* in run (glob)
    */mercurial/dispatch.py:* in dispatch (glob)
    */mercurial/dispatch.py:* in _runcatch (glob)
    */mercurial/dispatch.py:* in callcatch (glob)
    */mercurial/dispatch.py:* in _runcatchfunc (glob)
    */mercurial/dispatch.py:* in _dispatch (glob)
    */mercurial/dispatch.py:* in runcommand (glob)
    */mercurial/dispatch.py:* in _runcommand (glob)
-   */mercurial/dispatch.py:* in checkargs (glob)
    */mercurial/dispatch.py:* in <lambda> (glob)
    */mercurial/util.py:* in check (glob)
    $TESTTMP/buggylocking.py:* in buggylocking (glob)
   $ hg properlocking
   $ hg nowaitlocking
 
   $ echo a > a
   $ hg add a
@@ -127,17 +126,16 @@ 
    */mercurial/dispatch.py:* in run (glob)
    */mercurial/dispatch.py:* in dispatch (glob)
    */mercurial/dispatch.py:* in _runcatch (glob)
    */mercurial/dispatch.py:* in callcatch (glob)
    */mercurial/dispatch.py:* in _runcatchfunc (glob)
    */mercurial/dispatch.py:* in _dispatch (glob)
    */mercurial/dispatch.py:* in runcommand (glob)
    */mercurial/dispatch.py:* in _runcommand (glob)
-   */mercurial/dispatch.py:* in checkargs (glob)
    */mercurial/dispatch.py:* in <lambda> (glob)
    */mercurial/util.py:* in check (glob)
    $TESTTMP/buggylocking.py:* in oldanddeprecated (glob)
   $ hg blackbox -l 9
   1970/01/01 00:00:00 bob @cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b (5000)> devel-warn: revset "oldstyle" uses list instead of smartset
   (compatibility will be dropped after Mercurial-3.9, update your code.) at: *mercurial/revset.py:* (mfunc) (glob)
   1970/01/01 00:00:00 bob @cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b (5000)> log -r oldstyle() -T {rev}\n exited 0 after * seconds (glob)
   1970/01/01 00:00:00 bob @cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b (5000)> oldanddeprecated
@@ -151,17 +149,16 @@ 
    */mercurial/dispatch.py:* in run (glob)
    */mercurial/dispatch.py:* in dispatch (glob)
    */mercurial/dispatch.py:* in _runcatch (glob)
    */mercurial/dispatch.py:* in callcatch (glob)
    */mercurial/dispatch.py:* in _runcatchfunc (glob)
    */mercurial/dispatch.py:* in _dispatch (glob)
    */mercurial/dispatch.py:* in runcommand (glob)
    */mercurial/dispatch.py:* in _runcommand (glob)
-   */mercurial/dispatch.py:* in checkargs (glob)
    */mercurial/dispatch.py:* in <lambda> (glob)
    */mercurial/util.py:* in check (glob)
    $TESTTMP/buggylocking.py:* in oldanddeprecated (glob)
   1970/01/01 00:00:00 bob @cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b (5000)> oldanddeprecated --traceback exited 0 after * seconds (glob)
   1970/01/01 00:00:00 bob @cb9a9f314b8b07ba71012fcdbc544b5a4d82ff5b (5000)> blackbox -l 9
 
 Test programming error failure: