Patchwork [2,of,3] revset: add an undocumented _missingancestors function

login
register
mail settings
Submitter Siddharth Agarwal
Date Feb. 13, 2014, 10:10 p.m.
Message ID <b23315bb97257c5a8c2a.1392329431@dev1738.prn1.facebook.com>
Download mbox | patch
Permalink /patch/3638/
State Accepted
Commit fb2df4506c8716ac02d295172e00dd1bc9eb928c
Headers show

Comments

Siddharth Agarwal - Feb. 13, 2014, 10:10 p.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1392328485 28800
#      Thu Feb 13 13:54:45 2014 -0800
# Node ID b23315bb97257c5a8c2a6810990b19f5aa1dd2c7
# Parent  36f133144674e0f91b4c16d3ca49c01b3046f572
revset: add an undocumented _missingancestors function

This will be used in an upcoming patch to optimize expressions of the form
(::x - ::y).
Durham Goode - Feb. 14, 2014, 6:55 p.m.
On 2/13/14 2:10 PM, "Siddharth Agarwal" <sid0@fb.com> wrote:

># HG changeset patch
># User Siddharth Agarwal <sid0@fb.com>
># Date 1392328485 28800
>#      Thu Feb 13 13:54:45 2014 -0800
># Node ID b23315bb97257c5a8c2a6810990b19f5aa1dd2c7
># Parent  36f133144674e0f91b4c16d3ca49c01b3046f572
>revset: add an undocumented _missingancestors function
>
>This will be used in an upcoming patch to optimize expressions of the form
>(::x - ::y).
>
>diff --git a/mercurial/revset.py b/mercurial/revset.py
>--- a/mercurial/revset.py
>+++ b/mercurial/revset.py
>@@ -1021,6 +1021,16 @@
>             return baseset([m])
>     return baseset([])
> 
>+def _missingancestors(repo, subset, x):
>+    # i18n: "_missingancestors" is a keyword
>+    revs, bases = getargs(x, 2, 2,
>+                          _("_missingancestors requires two arguments"))
>+    rs = baseset(repo)
>+    revs = getset(repo, rs, revs)
>+    bases = getset(repo, rs, bases)
>+    missing = set(repo.changelog.findmissingrevs(bases, revs))
>+    return baseset([r for r in subset if r in missing])
>+

You could use the new 'lazyset' class for the return value here, so we
don't have to iterate over the entire subset immediately.
Siddharth Agarwal - Feb. 16, 2014, 6:41 a.m.
On 02/14/2014 10:55 AM, Durham Goode wrote:
> On 2/13/14 2:10 PM, "Siddharth Agarwal" <sid0@fb.com> wrote:
>
>> # HG changeset patch
>> # User Siddharth Agarwal <sid0@fb.com>
>> # Date 1392328485 28800
>> #      Thu Feb 13 13:54:45 2014 -0800
>> # Node ID b23315bb97257c5a8c2a6810990b19f5aa1dd2c7
>> # Parent  36f133144674e0f91b4c16d3ca49c01b3046f572
>> revset: add an undocumented _missingancestors function
>>
>> This will be used in an upcoming patch to optimize expressions of the form
>> (::x - ::y).
>>
>> diff --git a/mercurial/revset.py b/mercurial/revset.py
>> --- a/mercurial/revset.py
>> +++ b/mercurial/revset.py
>> @@ -1021,6 +1021,16 @@
>>              return baseset([m])
>>      return baseset([])
>>
>> +def _missingancestors(repo, subset, x):
>> +    # i18n: "_missingancestors" is a keyword
>> +    revs, bases = getargs(x, 2, 2,
>> +                          _("_missingancestors requires two arguments"))
>> +    rs = baseset(repo)
>> +    revs = getset(repo, rs, revs)
>> +    bases = getset(repo, rs, bases)
>> +    missing = set(repo.changelog.findmissingrevs(bases, revs))
>> +    return baseset([r for r in subset if r in missing])
>> +
> You could use the new 'lazyset' class for the return value here, so we
> don't have to iterate over the entire subset immediately.
>

I replaced the baseset line with

     return lazyset(subset, lambda r: r in missing)

and perfrevset on the revset '::tip - ::-10000' (performed in the next 
patch) is significantly worse:

! wall 0.232009 comb 0.240000 user 0.230000 sys 0.010000 (best of 42)

Looking at the profile, it seems like the entire set is materialized, so 
the lambda function calls only slow things down. Am I missing something?
Lucas Moscovicz - Feb. 16, 2014, 8:47 a.m.
On 2/15/14, 10:41 PM, "Siddharth Agarwal" <sid0@fb.com> wrote:

>On 02/14/2014 10:55 AM, Durham Goode wrote:
>> On 2/13/14 2:10 PM, "Siddharth Agarwal" <sid0@fb.com> wrote:
>>
>>> # HG changeset patch
>>> # User Siddharth Agarwal <sid0@fb.com>
>>> # Date 1392328485 28800
>>> #      Thu Feb 13 13:54:45 2014 -0800
>>> # Node ID b23315bb97257c5a8c2a6810990b19f5aa1dd2c7
>>> # Parent  36f133144674e0f91b4c16d3ca49c01b3046f572
>>> revset: add an undocumented _missingancestors function
>>>
>>> This will be used in an upcoming patch to optimize expressions of the
>>>form
>>> (::x - ::y).
>>>
>>> diff --git a/mercurial/revset.py b/mercurial/revset.py
>>> --- a/mercurial/revset.py
>>> +++ b/mercurial/revset.py
>>> @@ -1021,6 +1021,16 @@
>>>              return baseset([m])
>>>      return baseset([])
>>>
>>> +def _missingancestors(repo, subset, x):
>>> +    # i18n: "_missingancestors" is a keyword
>>> +    revs, bases = getargs(x, 2, 2,
>>> +                          _("_missingancestors requires two
>>>arguments"))
>>> +    rs = baseset(repo)
>>> +    revs = getset(repo, rs, revs)
>>> +    bases = getset(repo, rs, bases)
>>> +    missing = set(repo.changelog.findmissingrevs(bases, revs))
>>> +    return baseset([r for r in subset if r in missing])
>>> +
>> You could use the new 'lazyset' class for the return value here, so we
>> don't have to iterate over the entire subset immediately.
>>
>
>I replaced the baseset line with
>
>     return lazyset(subset, lambda r: r in missing)
>
>and perfrevset on the revset '::tip - ::-10000' (performed in the next
>patch) is significantly worse:
>
>! wall 0.232009 comb 0.240000 user 0.230000 sys 0.010000 (best of 42)
>
>Looking at the profile, it seems like the entire set is materialized, so
>the lambda function calls only slow things down. Am I missing something?

At this point the code in scmutil and cmdutil has not been changed yet to
work with lazy implementations and the entire set is still being
materialized there, so probably the only way to see a difference so far
would be if you try Œfirst(::tip - ::-10000)¹ which will return as fast as
the first value is produced.

I¹ve been working this days to change that code to be able to have those
lazy results without having to limit the expression.

>
>_______________________________________________
>Mercurial-devel mailing list
>Mercurial-devel@selenic.com
>https://urldefense.proofpoint.com/v1/url?u=http://selenic.com/mailman/list
>info/mercurial-devel&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=OvJpSDyvbZ%2BdRIG
>uE%2BQNXdEMu%2FMWX%2BVvreTVxvKUMnE%3D%0A&m=8%2BOCbHeWV4a8rg6JbUgRuyg%2FZl8
>EJqvDMtIzrQ1bMcQ%3D%0A&s=e30e3aebe547e8e628c0a96f1e1e8ae423714d6d88792a052
>634d59c198a39da

Patch

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -1021,6 +1021,16 @@ 
             return baseset([m])
     return baseset([])
 
+def _missingancestors(repo, subset, x):
+    # i18n: "_missingancestors" is a keyword
+    revs, bases = getargs(x, 2, 2,
+                          _("_missingancestors requires two arguments"))
+    rs = baseset(repo)
+    revs = getset(repo, rs, revs)
+    bases = getset(repo, rs, bases)
+    missing = set(repo.changelog.findmissingrevs(bases, revs))
+    return baseset([r for r in subset if r in missing])
+
 def modifies(repo, subset, x):
     """``modifies(pattern)``
     Changesets modifying files matched by pattern.
@@ -1602,6 +1612,7 @@ 
     "max": maxrev,
     "merge": merge,
     "min": minrev,
+    "_missingancestors": _missingancestors,
     "modifies": modifies,
     "obsolete": obsolete,
     "origin": origin,
@@ -1671,6 +1682,7 @@ 
     "max",
     "merge",
     "min",
+    "_missingancestors",
     "modifies",
     "obsolete",
     "origin",