[PATCH 1 of 3 v2] profiling: allow nested usage of maybeprofile

Arun Kulshreshtha kulshrax at fb.com
Thu Sep 22 05:04:40 EDT 2016




On 9/21/16, 8:51 PM, "Yuya Nishihara" <youjah at gmail.com on behalf of yuya at tcha.org> wrote:

    On Wed, 21 Sep 2016 18:07:57 +0000, Arun Kulshreshtha wrote:
    > On 9/21/16, 7:41 AM, "Yuya Nishihara" <yuya at tcha.org> wrote:
    >     > maybeprofile() can be called in threads. If we need to prevent nesting, we'll
    >     > have to save the flag in TLS.
    >     
    >     That said, I'm not sure if nested maybeprofile() can be noop. Last time, Greg
    >     tried to make statprof stackable, which would imply maybeprofile() designed
    >     to be nested.
    >     
    > Hm, right now it seems like if you stack profilers, you’ll get multiple profiling statistics printouts at the end of command execution, which is hard to read. But this this behavior is actually desirable, then we’ll need to deal with this situation differently.
    
    Perhaps we can move both maybeprofile() calls to _dispatch() without thinking
    about how nested profilers behave.
    
    --- a/mercurial/dispatch.py
    +++ b/mercurial/dispatch.py
    @@ -774,7 +774,8 @@ def _dispatch(req):
         # Check abbreviation/ambiguity of shell alias.
         shellaliasfn = _checkshellalias(lui, ui, args)
         if shellaliasfn:
    -        return shellaliasfn()
    +        with profiling.maybeprofile(lui):
    +            return shellaliasfn()
     
         # check for fallback encoding
         fallback = lui.config('ui', 'fallbackencoding')
    @@ -844,6 +845,10 @@ def _dispatch(req):
         elif not cmd:
             return commands.help_(ui, 'shortlist')
     
    +    with profiling.maybeprofile(lui):
    +        return _dispatchcommand(...)
    +
    +def _dispatchcommand(...):
         repo = None
         cmdpats = args[:]
         if not _cmdattr(ui, cmd, func, 'norepo'):
    

Yes, in fact this is originally what I had intended to do. However, this would mean that it
would not be possible to enable profiling from the repo-specific config file. Of course, we could
simply add a third call to maybeprofile after the call to hg.repository(), but prevent having duplicate
output with a boolean flag to check if profiling was already enabled before reposetup. This approach
seems a little inelegant, but it’s also simple enough.



More information about the Mercurial-devel mailing list