Patchwork [2,of,2] reachableroots: use baseset lazy sorting

login
register
mail settings
Submitter Pierre-Yves David
Date Aug. 21, 2015, 10:58 p.m.
Message ID <1635f6a58066e632e381.1440197922@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/10250/
State Accepted
Headers show

Comments

Pierre-Yves David - Aug. 21, 2015, 10:58 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1440116601 25200
#      Thu Aug 20 17:23:21 2015 -0700
# Node ID 1635f6a58066e632e3818cf9e9bf0c5aacb3e9f4
# Parent  76e8706080029160e4b71b043ac20b2760516eab
reachableroots: use baseset lazy sorting

smartset sorting is lazy (so faster in some case) and better (informs that the
set is sorted allowing some optimisation). So we rely on it directly.

Some test output are updated because we now have more information (ordering).
Yuya Nishihara - Aug. 22, 2015, 11:33 a.m.
On Fri, 21 Aug 2015 15:58:42 -0700, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@fb.com>
> # Date 1440116601 25200
> #      Thu Aug 20 17:23:21 2015 -0700
> # Node ID 1635f6a58066e632e3818cf9e9bf0c5aacb3e9f4
> # Parent  76e8706080029160e4b71b043ac20b2760516eab
> reachableroots: use baseset lazy sorting
> 
> smartset sorting is lazy (so faster in some case) and better (informs that the
> set is sorted allowing some optimisation). So we rely on it directly.
> 
> Some test output are updated because we now have more information (ordering).
> 
> diff --git a/mercurial/changelog.py b/mercurial/changelog.py
> --- a/mercurial/changelog.py
> +++ b/mercurial/changelog.py
> @@ -184,12 +184,14 @@ class changelog(revlog.revlog):
>          # XXX need filtering too
>          self.rev(self.node(0))
>          return self._nodecache
>  
>      def reachableroots(self, minroot, heads, roots, includepath=False):
> -        return revset.baseset(sorted(
> -            self.index.reachableroots(minroot, heads, roots, includepath)))
> +        rroots = self.index.reachableroots(minroot, heads, roots, includepath)
> +        rroots = revset.baseset(rroots)
> +        rroots.sort()
> +        return rroots

The series looks good to me.

This will save ~10% time of "0::tip" because baseset can sort list in place.
Augie Fackler - Aug. 24, 2015, 1:45 p.m.
On Sat, Aug 22, 2015 at 08:33:44PM +0900, Yuya Nishihara wrote:
> On Fri, 21 Aug 2015 15:58:42 -0700, Pierre-Yves David wrote:
> > # HG changeset patch
> > # User Pierre-Yves David <pierre-yves.david@fb.com>
> > # Date 1440116601 25200
> > #      Thu Aug 20 17:23:21 2015 -0700
> > # Node ID 1635f6a58066e632e3818cf9e9bf0c5aacb3e9f4
> > # Parent  76e8706080029160e4b71b043ac20b2760516eab
> > reachableroots: use baseset lazy sorting
> >
> > smartset sorting is lazy (so faster in some case) and better (informs that the
> > set is sorted allowing some optimisation). So we rely on it directly.
> >
> > Some test output are updated because we now have more information (ordering).
> >
> > diff --git a/mercurial/changelog.py b/mercurial/changelog.py
> > --- a/mercurial/changelog.py
> > +++ b/mercurial/changelog.py
> > @@ -184,12 +184,14 @@ class changelog(revlog.revlog):
> >          # XXX need filtering too
> >          self.rev(self.node(0))
> >          return self._nodecache
> >
> >      def reachableroots(self, minroot, heads, roots, includepath=False):
> > -        return revset.baseset(sorted(
> > -            self.index.reachableroots(minroot, heads, roots, includepath)))
> > +        rroots = self.index.reachableroots(minroot, heads, roots, includepath)
> > +        rroots = revset.baseset(rroots)
> > +        rroots.sort()
> > +        return rroots
>
> The series looks good to me.
>
> This will save ~10% time of "0::tip" because baseset can sort list in place.

Nice. Queued.

> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
Pierre-Yves David - Aug. 24, 2015, 10:41 p.m.
On 08/24/2015 06:45 AM, Augie Fackler wrote:
> On Sat, Aug 22, 2015 at 08:33:44PM +0900, Yuya Nishihara wrote:
>> On Fri, 21 Aug 2015 15:58:42 -0700, Pierre-Yves David wrote:
>>> # HG changeset patch
>>> # User Pierre-Yves David <pierre-yves.david@fb.com>
>>> # Date 1440116601 25200
>>> #      Thu Aug 20 17:23:21 2015 -0700
>>> # Node ID 1635f6a58066e632e3818cf9e9bf0c5aacb3e9f4
>>> # Parent  76e8706080029160e4b71b043ac20b2760516eab
>>> reachableroots: use baseset lazy sorting
>>>
>>> smartset sorting is lazy (so faster in some case) and better (informs that the
>>> set is sorted allowing some optimisation). So we rely on it directly.
>>>
>>> Some test output are updated because we now have more information (ordering).
>>>
>>> diff --git a/mercurial/changelog.py b/mercurial/changelog.py
>>> --- a/mercurial/changelog.py
>>> +++ b/mercurial/changelog.py
>>> @@ -184,12 +184,14 @@ class changelog(revlog.revlog):
>>>           # XXX need filtering too
>>>           self.rev(self.node(0))
>>>           return self._nodecache
>>>
>>>       def reachableroots(self, minroot, heads, roots, includepath=False):
>>> -        return revset.baseset(sorted(
>>> -            self.index.reachableroots(minroot, heads, roots, includepath)))
>>> +        rroots = self.index.reachableroots(minroot, heads, roots, includepath)
>>> +        rroots = revset.baseset(rroots)
>>> +        rroots.sort()
>>> +        return rroots
>>
>> The series looks good to me.
>>
>> This will save ~10% time of "0::tip" because baseset can sort list in place.
>
> Nice. Queued.

This break the --pure tests, I've a fix coming.

Patch

diff --git a/mercurial/changelog.py b/mercurial/changelog.py
--- a/mercurial/changelog.py
+++ b/mercurial/changelog.py
@@ -184,12 +184,14 @@  class changelog(revlog.revlog):
         # XXX need filtering too
         self.rev(self.node(0))
         return self._nodecache
 
     def reachableroots(self, minroot, heads, roots, includepath=False):
-        return revset.baseset(sorted(
-            self.index.reachableroots(minroot, heads, roots, includepath)))
+        rroots = self.index.reachableroots(minroot, heads, roots, includepath)
+        rroots = revset.baseset(rroots)
+        rroots.sort()
+        return rroots
 
     def headrevs(self):
         if self.filteredrevs:
             try:
                 return self.index.headrevsfiltered(self.filteredrevs)
diff --git a/tests/test-parseindex.t b/tests/test-parseindex.t
--- a/tests/test-parseindex.t
+++ b/tests/test-parseindex.t
@@ -80,13 +80,13 @@  Test SEGV caused by bad revision passed 
   >         print 'uncaught buffer overflow?'
   >     except (IndexError, TypeError) as inst:
   >         print inst
   > EOF
   goods:
-  0: <baseset [0]>
-  1: <baseset [0]>
-  -1: <baseset []>
+  0: <baseset+ [0]>
+  1: <baseset+ [0]>
+  -1: <baseset+ []>
   bads:
   2: head out of range
   10000: head out of range
   -2: head out of range
   -10000: head out of range
diff --git a/tests/test-revset.t b/tests/test-revset.t
--- a/tests/test-revset.t
+++ b/tests/test-revset.t
@@ -139,11 +139,11 @@  trivial
   $ try 3::6
   (dagrange
     ('symbol', '3')
     ('symbol', '6'))
   * set:
-  <baseset [3, 5, 6]>
+  <baseset+ [3, 5, 6]>
   3
   5
   6
   $ try '0|1|2'
   (or
@@ -989,11 +989,11 @@  test that `or` operation skips duplicate
     (func
       ('symbol', 'ancestors')
       ('symbol', '4')))
   * set:
   <addset
-    <baseset [5, 3, 1]>,
+    <baseset- [1, 3, 5]>,
     <generatorset+>>
   5
   3
   1
   0
@@ -1012,11 +1012,11 @@  test that `or` operation skips duplicate
           ('symbol', '1')
           ('symbol', '5')))))
   * set:
   <addset+
     <generatorset+>,
-    <baseset [5, 3, 1]>>
+    <baseset- [1, 3, 5]>>
   0
   1
   2
   3
   4