Patchwork [9,of,9,RFC] profiling: make statprof the default profiler (BC)

login
register
mail settings
Submitter Gregory Szorc
Date Aug. 16, 2016, 5:25 a.m.
Message ID <d88d80210ff4351734d6.1471325116@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/16314/
State Superseded
Headers show

Comments

Gregory Szorc - Aug. 16, 2016, 5:25 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1471221528 25200
#      Sun Aug 14 17:38:48 2016 -0700
# Node ID d88d80210ff4351734d63b50e1af75f398af8963
# Parent  1975493743c5f68f28bf9dcf677df09d0265581a
profiling: make statprof the default profiler (BC)

The statprof sampling profiler runs with significantly less overhead.
Its data is therefore more useful. Furthermore, its default output
shows the hotpath by default, which I've found to be way more useful
than the default profiler's function time table.
Augie Fackler - Aug. 17, 2016, 3:19 p.m.
On Mon, Aug 15, 2016 at 10:25:16PM -0700, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1471221528 25200
> #      Sun Aug 14 17:38:48 2016 -0700
> # Node ID d88d80210ff4351734d63b50e1af75f398af8963
> # Parent  1975493743c5f68f28bf9dcf677df09d0265581a
> profiling: make statprof the default profiler (BC)

I'm going to go ahead and queue this. It kind of bums me out to vendor
the whole profiler, but I guess this is probably enough of a better
diagnosis improvement for end users that we should just do it.

The management of https://github.com/bos/statprof.py (me!) would
appreciate sending any important improvements upstream. Thanks!

>
> The statprof sampling profiler runs with significantly less overhead.
> Its data is therefore more useful. Furthermore, its default output
> shows the hotpath by default, which I've found to be way more useful
> than the default profiler's function time table.
>
> diff --git a/mercurial/profiling.py b/mercurial/profiling.py
> --- a/mercurial/profiling.py
> +++ b/mercurial/profiling.py
> @@ -114,20 +114,20 @@ def statprofile(ui, fp):
>  def profile(ui):
>      """Start profiling.
>
>      Profiling is active when the context manager is active. When the context
>      manager exits, profiling results will be written to the configured output.
>      """
>      profiler = os.getenv('HGPROF')
>      if profiler is None:
> -        profiler = ui.config('profiling', 'type', default='ls')
> +        profiler = ui.config('profiling', 'type', default='stat')
>      if profiler not in ('ls', 'stat', 'flame'):
>          ui.warn(_("unrecognized profiler '%s' - ignored\n") % profiler)
> -        profiler = 'ls'
> +        profiler = 'stat'
>
>      output = ui.config('profiling', 'output')
>
>      if output == 'blackbox':
>          fp = util.stringio()
>      elif output:
>          path = ui.expandpath(output)
>          fp = open(path, 'wb')
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Augie Fackler - Aug. 17, 2016, 3:27 p.m.
On Wed, Aug 17, 2016 at 11:19:03AM -0400, Augie Fackler wrote:
> On Mon, Aug 15, 2016 at 10:25:16PM -0700, Gregory Szorc wrote:
> > # HG changeset patch
> > # User Gregory Szorc <gregory.szorc@gmail.com>
> > # Date 1471221528 25200
> > #      Sun Aug 14 17:38:48 2016 -0700
> > # Node ID d88d80210ff4351734d63b50e1af75f398af8963
> > # Parent  1975493743c5f68f28bf9dcf677df09d0265581a
> > profiling: make statprof the default profiler (BC)
>
> I'm going to go ahead and queue this. It kind of bums me out to vendor
> the whole profiler, but I guess this is probably enough of a better
> diagnosis improvement for end users that we should just do it.
>
> The management of https://github.com/bos/statprof.py (me!) would
> appreciate sending any important improvements upstream. Thanks!

Er, dropping these for now, on account of them breaking test-profile.t
and test-check-module-imports.t. The former in particular needs some
love before we move forward.

Thanks!

>
> >
> > The statprof sampling profiler runs with significantly less overhead.
> > Its data is therefore more useful. Furthermore, its default output
> > shows the hotpath by default, which I've found to be way more useful
> > than the default profiler's function time table.
> >
> > diff --git a/mercurial/profiling.py b/mercurial/profiling.py
> > --- a/mercurial/profiling.py
> > +++ b/mercurial/profiling.py
> > @@ -114,20 +114,20 @@ def statprofile(ui, fp):
> >  def profile(ui):
> >      """Start profiling.
> >
> >      Profiling is active when the context manager is active. When the context
> >      manager exits, profiling results will be written to the configured output.
> >      """
> >      profiler = os.getenv('HGPROF')
> >      if profiler is None:
> > -        profiler = ui.config('profiling', 'type', default='ls')
> > +        profiler = ui.config('profiling', 'type', default='stat')
> >      if profiler not in ('ls', 'stat', 'flame'):
> >          ui.warn(_("unrecognized profiler '%s' - ignored\n") % profiler)
> > -        profiler = 'ls'
> > +        profiler = 'stat'
> >
> >      output = ui.config('profiling', 'output')
> >
> >      if output == 'blackbox':
> >          fp = util.stringio()
> >      elif output:
> >          path = ui.expandpath(output)
> >          fp = open(path, 'wb')
> > _______________________________________________
> > Mercurial-devel mailing list
> > Mercurial-devel@mercurial-scm.org
> > https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/profiling.py b/mercurial/profiling.py
--- a/mercurial/profiling.py
+++ b/mercurial/profiling.py
@@ -114,20 +114,20 @@  def statprofile(ui, fp):
 def profile(ui):
     """Start profiling.
 
     Profiling is active when the context manager is active. When the context
     manager exits, profiling results will be written to the configured output.
     """
     profiler = os.getenv('HGPROF')
     if profiler is None:
-        profiler = ui.config('profiling', 'type', default='ls')
+        profiler = ui.config('profiling', 'type', default='stat')
     if profiler not in ('ls', 'stat', 'flame'):
         ui.warn(_("unrecognized profiler '%s' - ignored\n") % profiler)
-        profiler = 'ls'
+        profiler = 'stat'
 
     output = ui.config('profiling', 'output')
 
     if output == 'blackbox':
         fp = util.stringio()
     elif output:
         path = ui.expandpath(output)
         fp = open(path, 'wb')