Patchwork [11,of,11] revset: use `subset &` in bare `p2()`

login
register
mail settings
Submitter Pierre-Yves David
Date Sept. 23, 2014, 10:47 p.m.
Message ID <62f989f177ba1effe2b8.1411512477@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/5943/
State Superseded
Commit 6f434ef5422269dc95c315ccb899ef4d5b3c38b8
Headers show

Comments

Pierre-Yves David - Sept. 23, 2014, 10:47 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1410976803 25200
#      Wed Sep 17 11:00:03 2014 -0700
# Node ID 62f989f177ba1effe2b86833a1f0ca5715434675
# Parent  d2ac8f8e413275de8d61978f1653c8d1019e95da
revset: use `subset &` in bare `p2()`

This take advantage of the `fullreposet` smartness with a nice speedup.
Except similar speedup than for p1 when on a merge. (non merge are equally
lightning fast anyway)
Augie Fackler - Sept. 24, 2014, 5:14 p.m.
On Tue, Sep 23, 2014 at 03:47:57PM -0700, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1410976803 25200
> #      Wed Sep 17 11:00:03 2014 -0700
> # Node ID 62f989f177ba1effe2b86833a1f0ca5715434675
> # Parent  d2ac8f8e413275de8d61978f1653c8d1019e95da
> revset: use `subset &` in bare `p2()`

Queued the lot, with english tweaks in log messages.

>
> This take advantage of the `fullreposet` smartness with a nice speedup.
> Except similar speedup than for p1 when on a merge. (non merge are equally
> lightning fast anyway)
>
> diff --git a/mercurial/revset.py b/mercurial/revset.py
> --- a/mercurial/revset.py
> +++ b/mercurial/revset.py
> @@ -1215,11 +1215,13 @@ def p2(repo, subset, x):
>      """
>      if x is None:
>          ps = repo[x].parents()
>          try:
>              p = ps[1].rev()
> -            return subset.filter(lambda r: r == p)
> +            if p >= 0:
> +                return subset & baseset([p])
> +            return baseset([])
>          except IndexError:
>              return baseset([])
>
>      ps = set()
>      cl = repo.changelog
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - Sept. 24, 2014, 5:29 p.m.
On 09/24/2014 10:14 AM, Augie Fackler wrote:
> On Tue, Sep 23, 2014 at 03:47:57PM -0700, Pierre-Yves David wrote:
>> # HG changeset patch
>> # User Pierre-Yves David <pierre-yves.david@fb.com>
>> # Date 1410976803 25200
>> #      Wed Sep 17 11:00:03 2014 -0700
>> # Node ID 62f989f177ba1effe2b86833a1f0ca5715434675
>> # Parent  d2ac8f8e413275de8d61978f1653c8d1019e95da
>> revset: use `subset &` in bare `p2()`
>
> Queued the lot, with english tweaks in log messages.

You should probably base yourself onto on the current clowncopter tip as 
this "depends" of other improvement pushed there by Durham.
Augie Fackler - Sept. 24, 2014, 5:43 p.m.
On Wed, Sep 24, 2014 at 1:29 PM, Pierre-Yves David
<pierre-yves.david@ens-lyon.org> wrote:
>> Queued the lot, with english tweaks in log messages.
>
>
> You should probably base yourself onto on the current clowncopter tip as
> this "depends" of other improvement pushed there by Durham.


I thought mpm didn't want us to cross the streams?

(Tests pass fine, so when things merge it'll work out, no?)
Pierre-Yves David - Sept. 24, 2014, 5:48 p.m.
On 09/24/2014 10:43 AM, Augie Fackler wrote:
> On Wed, Sep 24, 2014 at 1:29 PM, Pierre-Yves David
> <pierre-yves.david@ens-lyon.org> wrote:
>>> Queued the lot, with english tweaks in log messages.
>>
>>
>> You should probably base yourself onto on the current clowncopter tip as
>> this "depends" of other improvement pushed there by Durham.
>
>
> I thought mpm didn't want us to cross the streams?

I think this stand in general. Making and exception when a single long 
series is reviewed by multiple people would make sense to me.

>
> (Tests pass fine, so when things merge it'll work out, no?)

I do not garantee the performance of the result.
Pierre-Yves David - Sept. 24, 2014, 5:50 p.m.
On 09/24/2014 10:48 AM, Pierre-Yves David wrote:
>
>
> On 09/24/2014 10:43 AM, Augie Fackler wrote:
>> On Wed, Sep 24, 2014 at 1:29 PM, Pierre-Yves David
>> <pierre-yves.david@ens-lyon.org> wrote:
>>>> Queued the lot, with english tweaks in log messages.
>>>
>>>
>>> You should probably base yourself onto on the current clowncopter tip as
>>> this "depends" of other improvement pushed there by Durham.
>>
>>
>> I thought mpm didn't want us to cross the streams?
>
> I think this stand in general. Making and exception when a single long
> series is reviewed by multiple people would make sense to me.
>
>>
>> (Tests pass fine, so when things merge it'll work out, no?)
>
> I do not garantee the performance of the result.

(and future performance testing of this series will be hard. Current 
performance testing of the rest of this series requires a merge which is 
inconvenient too)
Augie Fackler - Sept. 24, 2014, 6:47 p.m.
On Wed, Sep 24, 2014 at 1:50 PM, Pierre-Yves David
<pierre-yves.david@ens-lyon.org> wrote:
>
>
> On 09/24/2014 10:48 AM, Pierre-Yves David wrote:
>>
>>
>>
>> On 09/24/2014 10:43 AM, Augie Fackler wrote:
>>>
>>> On Wed, Sep 24, 2014 at 1:29 PM, Pierre-Yves David
>>> <pierre-yves.david@ens-lyon.org> wrote:
>>>>>
>>>>> Queued the lot, with english tweaks in log messages.
>>>>
>>>>
>>>>
>>>> You should probably base yourself onto on the current clowncopter tip as
>>>> this "depends" of other improvement pushed there by Durham.
>>>
>>>
>>>
>>> I thought mpm didn't want us to cross the streams?
>>
>>
>> I think this stand in general. Making and exception when a single long
>> series is reviewed by multiple people would make sense to me.
>>
>>>
>>> (Tests pass fine, so when things merge it'll work out, no?)
>>
>>
>> I do not garantee the performance of the result.
>
>
> (and future performance testing of this series will be hard. Current
> performance testing of the rest of this series requires a merge which is
> inconvenient too)


In that case, I've put my copyedited versions here for your clowncoptering:

http://hg.durin42.com/hg-queued/#marmoute-copyedit
David Soria Parra - Sept. 24, 2014, 8:43 p.m.
Augie Fackler <raf@durin42.com> writes:

> On Wed, Sep 24, 2014 at 1:50 PM, Pierre-Yves David
> <pierre-yves.david@ens-lyon.org> wrote:
>>
>>
>> On 09/24/2014 10:48 AM, Pierre-Yves David wrote:
>>>
>>>
>>>
>>> On 09/24/2014 10:43 AM, Augie Fackler wrote:
>>>>
>>>> On Wed, Sep 24, 2014 at 1:29 PM, Pierre-Yves David
>>>> <pierre-yves.david@ens-lyon.org> wrote:
>>>>>>
>>>>>> Queued the lot, with english tweaks in log messages.
>>>>>
>>>>>
>>>>>
>>>>> You should probably base yourself onto on the current clowncopter tip as
>>>>> this "depends" of other improvement pushed there by Durham.
>>>>
>>>>
>>>>
>>>> I thought mpm didn't want us to cross the streams?
>>>
>>>
>>> I think this stand in general. Making and exception when a single long
>>> series is reviewed by multiple people would make sense to me.
>>>
>>>>
>>>> (Tests pass fine, so when things merge it'll work out, no?)
>>>
>>>
>>> I do not garantee the performance of the result.
>>
>>
>> (and future performance testing of this series will be hard. Current
>> performance testing of the rest of this series requires a merge which is
>> inconvenient too)
>
>
> In that case, I've put my copyedited versions here for your clowncoptering:
>
> http://hg.durin42.com/hg-queued/#marmoute-copyedit

Rebased and pushed to clowncopter. I had to force draft phases,
apparently your server is publishing.
Pierre-Yves David - Sept. 24, 2014, 10:09 p.m.
On 09/24/2014 11:47 AM, Augie Fackler wrote:
> On Wed, Sep 24, 2014 at 1:50 PM, Pierre-Yves David
> <pierre-yves.david@ens-lyon.org> wrote:
>>
>>
>> On 09/24/2014 10:48 AM, Pierre-Yves David wrote:
>>>
>>>
>>>
>>> On 09/24/2014 10:43 AM, Augie Fackler wrote:
>>>>
>>>> On Wed, Sep 24, 2014 at 1:29 PM, Pierre-Yves David
>>>> <pierre-yves.david@ens-lyon.org> wrote:
>>>>>>
>>>>>> Queued the lot, with english tweaks in log messages.
>>>>>
>>>>>
>>>>>
>>>>> You should probably base yourself onto on the current clowncopter tip as
>>>>> this "depends" of other improvement pushed there by Durham.
>>>>
>>>>
>>>>
>>>> I thought mpm didn't want us to cross the streams?
>>>
>>>
>>> I think this stand in general. Making and exception when a single long
>>> series is reviewed by multiple people would make sense to me.
>>>
>>>>
>>>> (Tests pass fine, so when things merge it'll work out, no?)
>>>
>>>
>>> I do not garantee the performance of the result.
>>
>>
>> (and future performance testing of this series will be hard. Current
>> performance testing of the rest of this series requires a merge which is
>> inconvenient too)
>
>
> In that case, I've put my copyedited versions here for your clowncoptering:
>
> http://hg.durin42.com/hg-queued/#marmoute-copyedit

For some strange reason. I cannot pull.

% 14:58 pyd@marginatus ~/src/mercurial-dev > hg pull 
'http://hg.durin42.com/hg-queued/#marmoute-copyedit' --debug
using http://hg.durin42.com/hg-queued/
sending capabilities command
pulling from http://hg.durin42.com/hg-queued/
sending branchmap command
preparing listkeys for "bookmarks"
sending listkeys command
sending lookup command
query 1; heads
sending batch command
abort: HTTP Error 400: Bad Request
zsh: exit 255   hg pull 
'http://hg.durin42.com/hg-queued/#marmoute-copyedit' --debug

neither clone:

% 14:59 pyd@marginatus /tmp > hg clone http://hg.durin42.com/hg-queued/
destination directory: hg-queued
requesting all changes
adding changesets
adding manifests 

transaction abort!
rollback completed
abort: stream ended unexpectedly (got 2854 bytes, expected 9769)
zsh: exit 255   hg clone http://hg.durin42.com/hg-queued/

The plan sound good otherwise.

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -1215,11 +1215,13 @@  def p2(repo, subset, x):
     """
     if x is None:
         ps = repo[x].parents()
         try:
             p = ps[1].rev()
-            return subset.filter(lambda r: r == p)
+            if p >= 0:
+                return subset & baseset([p])
+            return baseset([])
         except IndexError:
             return baseset([])
 
     ps = set()
     cl = repo.changelog