Patchwork [3,of,3,v3] dispatch: make hg --profile wrap reposetup

login
register
mail settings
Submitter Arun Kulshreshtha
Date Sept. 20, 2016, 8:35 p.m.
Message ID <f157d9becfe724d624e0.1474403722@dev10559.prn2.facebook.com>
Download mbox | patch
Permalink /patch/16696/
State Accepted
Headers show

Comments

Arun Kulshreshtha - Sept. 20, 2016, 8:35 p.m.
# HG changeset patch
# User Arun Kulshreshtha <kulshrax@fb.com>
# Date 1474318006 25200
#      Mon Sep 19 13:46:46 2016 -0700
# Node ID f157d9becfe724d624e0f3328febe58c5694d950
# Parent  0bb030bb1ce94c5514b671a3900ba207ebb19269
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.
Augie Fackler - Sept. 21, 2016, 5:31 p.m.
On Tue, Sep 20, 2016 at 01:35:22PM -0700, Arun Kulshreshtha wrote:
> # HG changeset patch
> # User Arun Kulshreshtha <kulshrax@fb.com>
> # Date 1474318006 25200
> #      Mon Sep 19 13:46:46 2016 -0700
> # Node ID f157d9becfe724d624e0f3328febe58c5694d950
> # Parent  0bb030bb1ce94c5514b671a3900ba207ebb19269
> dispatch: make hg --profile wrap reposetup

This makes sense to me. Queued.

(I shiver a little bit at threadlocals, but what can you do? Sigh.)

>
> 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
Pierre-Yves David - Sept. 21, 2016, 5:50 p.m.
On 09/21/2016 07:31 PM, Augie Fackler wrote:
> On Tue, Sep 20, 2016 at 01:35:22PM -0700, Arun Kulshreshtha wrote:
>> # HG changeset patch
>> # User Arun Kulshreshtha <kulshrax@fb.com>
>> # Date 1474318006 25200
>> #      Mon Sep 19 13:46:46 2016 -0700
>> # Node ID f157d9becfe724d624e0f3328febe58c5694d950
>> # Parent  0bb030bb1ce94c5514b671a3900ba207ebb19269
>> dispatch: make hg --profile wrap reposetup
>
> This makes sense to me. Queued.
>
> (I shiver a little bit at threadlocals, but what can you do? Sigh.)

Yuya raised multiples valid question when reviewing V2 and it is unclear 
they have all been addressed especially since the discussion where still 
going on in V2 after V3 was sent).
Did you checked the status of this series with Yuya ?

Cheers,
Arun Kulshreshtha - Sept. 21, 2016, 6:04 p.m.
V3 doesn’t address all of the feedback on V2, since Yuya sent some additional comments after I’d already sent out V3. So this isn’t ready to land yet.


On 9/21/16, 10:50 AM, "Pierre-Yves David" <pierre-yves.david@ens-lyon.org> wrote:

    
    
    On 09/21/2016 07:31 PM, Augie Fackler wrote:
    > On Tue, Sep 20, 2016 at 01:35:22PM -0700, Arun Kulshreshtha wrote:

    >> # HG changeset patch

    >> # User Arun Kulshreshtha <kulshrax@fb.com>

    >> # Date 1474318006 25200

    >> #      Mon Sep 19 13:46:46 2016 -0700

    >> # Node ID f157d9becfe724d624e0f3328febe58c5694d950

    >> # Parent  0bb030bb1ce94c5514b671a3900ba207ebb19269

    >> dispatch: make hg --profile wrap reposetup

    >

    > This makes sense to me. Queued.

    >

    > (I shiver a little bit at threadlocals, but what can you do? Sigh.)

    
    Yuya raised multiples valid question when reviewing V2 and it is unclear 
    they have all been addressed especially since the discussion where still 
    going on in V2 after V3 was sent).
    Did you checked the status of this series with Yuya ?
    
    Cheers,
    
    -- 
    Pierre-Yves David
Augie Fackler - Sept. 21, 2016, 6:08 p.m.
> On Sep 21, 2016, at 13:50, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:
> 
> 
> 
> On 09/21/2016 07:31 PM, Augie Fackler wrote:
>> On Tue, Sep 20, 2016 at 01:35:22PM -0700, Arun Kulshreshtha wrote:
>>> # HG changeset patch
>>> # User Arun Kulshreshtha <kulshrax@fb.com>
>>> # Date 1474318006 25200
>>> #      Mon Sep 19 13:46:46 2016 -0700
>>> # Node ID f157d9becfe724d624e0f3328febe58c5694d950
>>> # Parent  0bb030bb1ce94c5514b671a3900ba207ebb19269
>>> dispatch: make hg --profile wrap reposetup
>> 
>> This makes sense to me. Queued.
>> 
>> (I shiver a little bit at threadlocals, but what can you do? Sigh.)
> 
> Yuya raised multiples valid question when reviewing V2 and it is unclear they have all been addressed especially since the discussion where still going on in V2 after V3 was sent).
> Did you checked the status of this series with Yuya ?

No, because I saw a v3 and discarded the entire v2 discussion, assuming v3 resolved the things discussed in v2.

> 
> Cheers,
> 
> -- 
> Pierre-Yves David
Arun Kulshreshtha - Sept. 21, 2016, 9:05 p.m.
On 9/21/16, 11:08 AM, "Augie Fackler" <raf@durin42.com> wrote:

    
    > On Sep 21, 2016, at 13:50, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:

    > 

    > 

    > 

    > On 09/21/2016 07:31 PM, Augie Fackler wrote:

    >> On Tue, Sep 20, 2016 at 01:35:22PM -0700, Arun Kulshreshtha wrote:

    >>> # HG changeset patch

    >>> # User Arun Kulshreshtha <kulshrax@fb.com>

    >>> # Date 1474318006 25200

    >>> #      Mon Sep 19 13:46:46 2016 -0700

    >>> # Node ID f157d9becfe724d624e0f3328febe58c5694d950

    >>> # Parent  0bb030bb1ce94c5514b671a3900ba207ebb19269

    >>> dispatch: make hg --profile wrap reposetup

    >> 

    >> This makes sense to me. Queued.

    >> 

    >> (I shiver a little bit at threadlocals, but what can you do? Sigh.)

    > 

    > Yuya raised multiples valid question when reviewing V2 and it is unclear they have all been addressed especially since the discussion where still going on in V2 after V3 was sent).

    > Did you checked the status of this series with Yuya ?

    
    No, because I saw a v3 and discarded the entire v2 discussion, assuming v3 resolved the things discussed in v2.
    
    > 

    > Cheers,

    > 

    > -- 

    > Pierre-Yves David

    
    
Is it possible to undo the accept? To repeat, Yuya made further comments on v2
after I had already sent out v3 addressing his first set of comments. This version 
doesn’t address the latest feedback so it shouldn’t be published yet.

Sorry about the confusion.
Augie Fackler - Sept. 21, 2016, 9:09 p.m.
> On Sep 21, 2016, at 17:05, Arun Kulshreshtha <kulshrax@fb.com> wrote:
> 
> 
> 
> 
> On 9/21/16, 11:08 AM, "Augie Fackler" <raf@durin42.com> wrote:
> 
> 
>> On Sep 21, 2016, at 13:50, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:
>> 
>> 
>> 
>> On 09/21/2016 07:31 PM, Augie Fackler wrote:
>>> On Tue, Sep 20, 2016 at 01:35:22PM -0700, Arun Kulshreshtha wrote:
>>>> # HG changeset patch
>>>> # User Arun Kulshreshtha <kulshrax@fb.com>
>>>> # Date 1474318006 25200
>>>> #      Mon Sep 19 13:46:46 2016 -0700
>>>> # Node ID f157d9becfe724d624e0f3328febe58c5694d950
>>>> # Parent  0bb030bb1ce94c5514b671a3900ba207ebb19269
>>>> dispatch: make hg --profile wrap reposetup
>>> 
>>> This makes sense to me. Queued.
>>> 
>>> (I shiver a little bit at threadlocals, but what can you do? Sigh.)
>> 
>> Yuya raised multiples valid question when reviewing V2 and it is unclear they have all been addressed especially since the discussion where still going on in V2 after V3 was sent).
>> Did you checked the status of this series with Yuya ?
> 
>    No, because I saw a v3 and discarded the entire v2 discussion, assuming v3 resolved the things discussed in v2.
> 
>> 
>> Cheers,
>> 
>> -- 
>> Pierre-Yves David
> 
> 
> Is it possible to undo the accept? To repeat, Yuya made further comments on v2
> after I had already sent out v3 addressing his first set of comments. This version 
> doesn’t address the latest feedback so it shouldn’t be published yet.
> 
> Sorry about the confusion.

Ugh, I thought I had done that, but it turns out obsolete markers and bookmarks play poorly in a way I didn't know about. It's fixed now.
Arun Kulshreshtha - Sept. 21, 2016, 9:10 p.m.
On 9/21/16, 2:05 PM, "Arun Kulshreshtha" <kulshrax@fb.com> wrote:

    
    
    
    On 9/21/16, 11:08 AM, "Augie Fackler" <raf@durin42.com> wrote:
    
        
        > On Sep 21, 2016, at 13:50, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:

        > 

        > 

        > 

        > On 09/21/2016 07:31 PM, Augie Fackler wrote:

        >> On Tue, Sep 20, 2016 at 01:35:22PM -0700, Arun Kulshreshtha wrote:

        >>> # HG changeset patch

        >>> # User Arun Kulshreshtha <kulshrax@fb.com>

        >>> # Date 1474318006 25200

        >>> #      Mon Sep 19 13:46:46 2016 -0700

        >>> # Node ID f157d9becfe724d624e0f3328febe58c5694d950

        >>> # Parent  0bb030bb1ce94c5514b671a3900ba207ebb19269

        >>> dispatch: make hg --profile wrap reposetup

        >> 

        >> This makes sense to me. Queued.

        >> 

        >> (I shiver a little bit at threadlocals, but what can you do? Sigh.)

        > 

        > Yuya raised multiples valid question when reviewing V2 and it is unclear they have all been addressed especially since the discussion where still going on in V2 after V3 was sent).

        > Did you checked the status of this series with Yuya ?

        
        No, because I saw a v3 and discarded the entire v2 discussion, assuming v3 resolved the things discussed in v2.
        
        > 

        > Cheers,

        > 

        > -- 

        > Pierre-Yves David

        
        
    Is it possible to undo the accept? To repeat, Yuya made further comments on v2
    after I had already sent out v3 addressing his first set of comments. This version 
    doesn’t address the latest feedback so it shouldn’t be published yet.
    
    Sorry about the confusion.
    
Never mind, looks like this was resolved over IRC.
Pierre-Yves David - Sept. 23, 2016, 1:40 a.m.
On 09/21/2016 11:09 PM, Augie Fackler wrote:
>
>> On Sep 21, 2016, at 17:05, Arun Kulshreshtha <kulshrax@fb.com> wrote:
>>
>>
>>
>>
>> On 9/21/16, 11:08 AM, "Augie Fackler" <raf@durin42.com> wrote:
>>
>>
>>> On Sep 21, 2016, at 13:50, Pierre-Yves David <pierre-yves.david@ens-lyon.org> wrote:
>>>
>>>
>>>
>>> On 09/21/2016 07:31 PM, Augie Fackler wrote:
>>>> On Tue, Sep 20, 2016 at 01:35:22PM -0700, Arun Kulshreshtha wrote:
>>>>> # HG changeset patch
>>>>> # User Arun Kulshreshtha <kulshrax@fb.com>
>>>>> # Date 1474318006 25200
>>>>> #      Mon Sep 19 13:46:46 2016 -0700
>>>>> # Node ID f157d9becfe724d624e0f3328febe58c5694d950
>>>>> # Parent  0bb030bb1ce94c5514b671a3900ba207ebb19269
>>>>> dispatch: make hg --profile wrap reposetup
>>>>
>>>> This makes sense to me. Queued.
>>>>
>>>> (I shiver a little bit at threadlocals, but what can you do? Sigh.)
>>>
>>> Yuya raised multiples valid question when reviewing V2 and it is unclear they have all been addressed especially since the discussion where still going on in V2 after V3 was sent).
>>> Did you checked the status of this series with Yuya ?
>>
>> No, because I saw a v3 and discarded the entire v2 discussion, assuming v3 resolved the things discussed in v2.

That assumption is probably a bit too quick. New contributors have a 
tendency to be very eager when it comes to send the next iteration, 
often before the previous batch is done discussing. In all case I 
usually find it useful to quickly checks the points made in the previous 
version by other reviewers. This help checking if they were all/properly 
fixed in the next iterations and unifying our pool of concerns.

Cheers,

Patch

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'):