Patchwork [4,of,5] revset: make head() honor order of subset

login
register
mail settings
Submitter via Mercurial-devel
Date June 24, 2016, 10:10 p.m.
Message ID <87950985ede58fc03fb4.1466806216@martinvonz.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/15610/
State Accepted
Headers show

Comments

via Mercurial-devel - June 24, 2016, 10:10 p.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@google.com>
# Date 1466710629 25200
#      Thu Jun 23 12:37:09 2016 -0700
# Node ID 87950985ede58fc03fb4a207cd45f197f1690cb8
# Parent  4fc03f313a06f6f1ba9638d2692351de064e8482
revset: make head() honor order of subset

The ordering of 'x & head()' was broken in 6a1a4c212d50 (revset:
improve head revset performance, 2014-03-13). Presumably due to other
optimizations since then, undoing that change to fix the order does
not slow down the simple case of "hg log -r 'head()'" mentioned in
that commit. I see a small slowdown from ~0.16s to about ~0.19s with
'not 0 & head()', but I'd say it's worth it for the correct output.
Yuya Nishihara - June 25, 2016, 4:13 a.m.
On Fri, 24 Jun 2016 15:10:16 -0700, Martin von Zweigbergk via Mercurial-devel wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1466710629 25200
> #      Thu Jun 23 12:37:09 2016 -0700
> # Node ID 87950985ede58fc03fb4a207cd45f197f1690cb8
> # Parent  4fc03f313a06f6f1ba9638d2692351de064e8482
> revset: make head() honor order of subset
> 
> The ordering of 'x & head()' was broken in 6a1a4c212d50 (revset:
> improve head revset performance, 2014-03-13). Presumably due to other
> optimizations since then, undoing that change to fix the order does
> not slow down the simple case of "hg log -r 'head()'" mentioned in
> that commit. I see a small slowdown from ~0.16s to about ~0.19s with
> 'not 0 & head()', but I'd say it's worth it for the correct output.

Yeah, fullreposet.__and__() serves for it. As long as a large set isn't
created explicitly, 'subset & baseset(hs)' should be comparable to
'baseset(hs) & subset'.

Unfair example to show slowdown:

revset #0: 0:tip & head()
   plain
0) 0.000236
1) 0.012398  x52

I'll queue 1, 3, 4, thanks.

Patch

diff -r 4fc03f313a06 -r 87950985ede5 mercurial/revset.py
--- a/mercurial/revset.py	Thu Jun 23 13:08:10 2016 -0700
+++ b/mercurial/revset.py	Thu Jun 23 12:37:09 2016 -0700
@@ -1145,9 +1145,7 @@ 
     cl = repo.changelog
     for ls in repo.branchmap().itervalues():
         hs.update(cl.rev(h) for h in ls)
-    # XXX We should combine with subset first: 'subset & baseset(...)'. This is
-    # necessary to ensure we preserve the order in subset.
-    return baseset(hs) & subset
+    return subset & baseset(hs)
 
 @predicate('heads(set)', safe=True)
 def heads(repo, subset, x):
diff -r 4fc03f313a06 -r 87950985ede5 tests/test-revset.t
--- a/tests/test-revset.t	Thu Jun 23 13:08:10 2016 -0700
+++ b/tests/test-revset.t	Thu Jun 23 12:37:09 2016 -0700
@@ -952,13 +952,12 @@ 
   1
   0
 
- 'head()' combines sets in wrong order:
+ 'head()' combines sets in right order:
 
   $ log '2:0 & head()'
-  0
+  2
   1
-  2
- BROKEN: should be '2 1 0'
+  0
 
  'a + b', which is optimized to '_list(a b)', should take the ordering of
  the left expression: