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

login
register
mail settings
Submitter Gregory Szorc
Date Aug. 15, 2016, 12:03 a.m.
Message ID <7d945851998c89444a8a.1471219436@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/16293/
State Superseded
Headers show

Comments

Gregory Szorc - Aug. 15, 2016, 12:03 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1471217701 25200
#      Sun Aug 14 16:35:01 2016 -0700
# Node ID 7d945851998c89444a8a53ac7daf1e0798cb7838
# Parent  cf7b933cbc7fd0dfc4c5d5d67deae2d52866088a
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).

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/profiling.py b/mercurial/profiling.py
--- a/mercurial/profiling.py
+++ b/mercurial/profiling.py
@@ -137,8 +137,25 @@  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.
+    """
+    if ui.configbool('profiling', 'enabled'):
+        with profile(ui):
+            yield
+    else:
+        yield