Re: [PATCH 3 of 3 v2] dispatch: make hg --profile wrap reposetup

[Durham Goode]

On 9/21/16 1:07 PM, Arun Kulshreshtha wrote:



On 9/21/16, 7:34 AM, "Yuya Nishihara"  wrote:

 On Tue, 20 Sep 2016 18:30:49 +, Arun Kulshreshtha wrote:
 > On 9/20/16, 6:01 AM, "Yuya Nishihara"  wrote:
 >
 > On Mon, 19 Sep 2016 16:13:58 -0700, Arun Kulshreshtha wrote:
 > > # HG changeset patch
 > > # User Arun Kulshreshtha 
 > > # Date 1474318006 25200
 > > #  Mon Sep 19 13:46:46 2016 -0700
 > > # Node ID 20af15cac045b249aece42cb71b671302b6c314c
 > > # Parent  6f33cc84cdd6c9ab38d32784505b6fb53bf3eba9
 > > dispatch: make hg --profile wrap reposetup
 > >
 > > Add profiling to _dispatch so that reposetup is included in the 
profiler
 > > output. All existing usage of the profiling context manager has 
been preserved,
 > > so the existing behavior of profiling enabled after reposetup will 
not be
 > > affected.
 > >
 > > diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
 > > --- a/mercurial/dispatch.py
 > > +++ b/mercurial/dispatch.py
 > > @@ -844,7 +844,7 @@
 > >  elif not cmd:
 > >  return commands.help_(ui, 'shortlist')
 > >
 > > -if True:
 > > +with profiling.maybeprofile(ui):
 > >  repo = None
 > >  cmdpats = args[:]
 > >  if not _cmdattr(ui, cmd, func, 'norepo'):
 >
 > Any reason to not remove maybeprofile() from _runcommand() ? Can it 
be enabled
 > after reposetup() ?
 >
 > Yes, if it is configured in the repo-specific settings (.hg/hgrc), for 
example, then it would be missed if
 > maybeprofile were removed from _runcommand().
 
 .hg/hgrc should be loaded to 'lui'. Can you check if maybeprofile(lui) works?
 
Alright, I’ll see if I can get that to work.


 > Additionally, we’d need to wrap other callsites of
 > _runcommand(), such as _checkshellalias(), to maintain the existing 
behavior.
 
 Good point. Given that we want to start profiling as early as possible, I think

 it's better to test profiling.enabled before extensions.loadall(lui), and 
test
 it again after parsing command options.

CC’ing Durham on this. I considered doing this, but during our (offline) 
discussion about this, it seemed
like it was undesirable to profile uisetup() for each extension, which is why I 
placed maybeprofile() after
extensions.loadall(). However, if we do indeed want to start profiling as early 
as possible, then I’ll try enabling
profiling as early as possible in _dispatch() and remove the other callsites.
The only reason I hesitated to wrap uisetup and extsetup is that 
extensions may modify the profile logic (like we have a statprofext 
extension that configures statprof), so wrapping early may make that 
impossible.

___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 3 of 3 v2] dispatch: make hg --profile wrap reposetup

[Arun Kulshreshtha]

On 9/21/16, 7:34 AM, "Yuya Nishihara"  wrote:

On Tue, 20 Sep 2016 18:30:49 +, Arun Kulshreshtha wrote:
> On 9/20/16, 6:01 AM, "Yuya Nishihara"  wrote:
> 
> On Mon, 19 Sep 2016 16:13:58 -0700, Arun Kulshreshtha wrote:
> > # HG changeset patch
> > # User Arun Kulshreshtha 
> > # Date 1474318006 25200
> > #  Mon Sep 19 13:46:46 2016 -0700
> > # Node ID 20af15cac045b249aece42cb71b671302b6c314c
> > # Parent  6f33cc84cdd6c9ab38d32784505b6fb53bf3eba9
> > dispatch: make hg --profile wrap reposetup
> > 
> > Add profiling to _dispatch so that reposetup is included in the 
profiler
> > output. All existing usage of the profiling context manager has 
been preserved,
> > so the existing behavior of profiling enabled after reposetup will 
not be
> > affected.
> > 
> > diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
> > --- a/mercurial/dispatch.py
> > +++ b/mercurial/dispatch.py
> > @@ -844,7 +844,7 @@
> >  elif not cmd:
> >  return commands.help_(ui, 'shortlist')
> >  
> > -if True:
> > +with profiling.maybeprofile(ui):
> >  repo = None
> >  cmdpats = args[:]
> >  if not _cmdattr(ui, cmd, func, 'norepo'):
> 
> Any reason to not remove maybeprofile() from _runcommand() ? Can it 
be enabled
> after reposetup() ?
> 
> Yes, if it is configured in the repo-specific settings (.hg/hgrc), for 
example, then it would be missed if
> maybeprofile were removed from _runcommand().

.hg/hgrc should be loaded to 'lui'. Can you check if maybeprofile(lui) 
works?

Alright, I’ll see if I can get that to work.

> Additionally, we’d need to wrap other callsites of
> _runcommand(), such as _checkshellalias(), to maintain the existing 
behavior.

Good point. Given that we want to start profiling as early as possible, I 
think
it's better to test profiling.enabled before extensions.loadall(lui), and 
test
it again after parsing command options.

CC’ing Durham on this. I considered doing this, but during our (offline) 
discussion about this, it seemed 
like it was undesirable to profile uisetup() for each extension, which is why I 
placed maybeprofile() after 
extensions.loadall(). However, if we do indeed want to start profiling as early 
as possible, then I’ll try enabling 
profiling as early as possible in _dispatch() and remove the other callsites.



___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 3 of 3 v2] dispatch: make hg --profile wrap reposetup

[Yuya Nishihara]

On Tue, 20 Sep 2016 18:30:49 +, Arun Kulshreshtha wrote:
> On 9/20/16, 6:01 AM, "Yuya Nishihara"  y...@tcha.org> wrote:
> 
> On Mon, 19 Sep 2016 16:13:58 -0700, Arun Kulshreshtha wrote:
> > # HG changeset patch
> > # User Arun Kulshreshtha 
> > # Date 1474318006 25200
> > #  Mon Sep 19 13:46:46 2016 -0700
> > # Node ID 20af15cac045b249aece42cb71b671302b6c314c
> > # Parent  6f33cc84cdd6c9ab38d32784505b6fb53bf3eba9
> > dispatch: make hg --profile wrap reposetup
> > 
> > Add profiling to _dispatch so that reposetup is included in the profiler
> > output. All existing usage of the profiling context manager has been 
> preserved,
> > so the existing behavior of profiling enabled after reposetup will not 
> be
> > affected.
> > 
> > diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
> > --- a/mercurial/dispatch.py
> > +++ b/mercurial/dispatch.py
> > @@ -844,7 +844,7 @@
> >  elif not cmd:
> >  return commands.help_(ui, 'shortlist')
> >  
> > -if True:
> > +with profiling.maybeprofile(ui):
> >  repo = None
> >  cmdpats = args[:]
> >  if not _cmdattr(ui, cmd, func, 'norepo'):
> 
> Any reason to not remove maybeprofile() from _runcommand() ? Can it be 
> enabled
> after reposetup() ?
> 
> Yes, if it is configured in the repo-specific settings (.hg/hgrc), for 
> example, then it would be missed if
> maybeprofile were removed from _runcommand().

.hg/hgrc should be loaded to 'lui'. Can you check if maybeprofile(lui) works?

> Additionally, we’d need to wrap other callsites of
> _runcommand(), such as _checkshellalias(), to maintain the existing behavior.

Good point. Given that we want to start profiling as early as possible, I think
it's better to test profiling.enabled before extensions.loadall(lui), and test
it again after parsing command options.
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Re: [PATCH 3 of 3 v2] dispatch: make hg --profile wrap reposetup

[Arun Kulshreshtha]

On 9/20/16, 6:01 AM, "Yuya Nishihara"  wrote:

On Mon, 19 Sep 2016 16:13:58 -0700, Arun Kulshreshtha wrote:
> # HG changeset patch
> # User Arun Kulshreshtha 
> # Date 1474318006 25200
> #  Mon Sep 19 13:46:46 2016 -0700
> # Node ID 20af15cac045b249aece42cb71b671302b6c314c
> # Parent  6f33cc84cdd6c9ab38d32784505b6fb53bf3eba9
> dispatch: make hg --profile wrap reposetup
> 
> Add profiling to _dispatch so that reposetup is included in the profiler
> output. All existing usage of the profiling context manager has been 
preserved,
> so the existing behavior of profiling enabled after reposetup will not be
> affected.
> 
> diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
> --- a/mercurial/dispatch.py
> +++ b/mercurial/dispatch.py
> @@ -844,7 +844,7 @@
>  elif not cmd:
>  return commands.help_(ui, 'shortlist')
>  
> -if True:
> +with profiling.maybeprofile(ui):
>  repo = None
>  cmdpats = args[:]
>  if not _cmdattr(ui, cmd, func, 'norepo'):

Any reason to not remove maybeprofile() from _runcommand() ? Can it be 
enabled
after reposetup() ?

Yes, if it is configured in the repo-specific settings (.hg/hgrc), for example, 
then it would be missed if
maybeprofile were removed from _runcommand(). Additionally, we’d need to wrap 
other callsites of
_runcommand(), such as _checkshellalias(), to maintain the existing behavior.

(Resending this because I didn’t CC the list; sorry for the duplicate message.)



___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

[PATCH 3 of 3 v2] dispatch: make hg --profile wrap reposetup

[Arun Kulshreshtha]

# HG changeset patch
# User Arun Kulshreshtha 
# Date 1474318006 25200
#  Mon Sep 19 13:46:46 2016 -0700
# Node ID 20af15cac045b249aece42cb71b671302b6c314c
# Parent  6f33cc84cdd6c9ab38d32784505b6fb53bf3eba9
dispatch: make hg --profile wrap reposetup

Add profiling to _dispatch so that reposetup is included in the profiler
output. All existing usage of the profiling context manager has been preserved,
so the existing behavior of profiling enabled after reposetup will not be
affected.

diff --git a/mercurial/dispatch.py b/mercurial/dispatch.py
--- a/mercurial/dispatch.py
+++ b/mercurial/dispatch.py
@@ -844,7 +844,7 @@
 elif not cmd:
 return commands.help_(ui, 'shortlist')
 
-if True:
+with profiling.maybeprofile(ui):
 repo = None
 cmdpats = args[:]
 if not _cmdattr(ui, cmd, func, 'norepo'):
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel