<div dir="ltr">For the record, Pierre-Yves and I talked about this more on IRC. I ended up pushing a "return early when revs is empty" patch to the clowncopter after Pierre-Yves reviewed it off the list (probably transient link: <a href="https://bitbucket.org/martinvonz/hg/commits/025000e6f13871a4eb3f6e49752d88c4785c009d">https://bitbucket.org/martinvonz/hg/commits/025000e6f13871a4eb3f6e49752d88c4785c009d</a>). Pierre-Yves will rebase and send a V2, IIUC.<br></div><br><div class="gmail_quote">On Wed, May 6, 2015 at 11:16 AM Martin von Zweigbergk <<a href="mailto:martinvonz@google.com">martinvonz@google.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_quote">On Mon, May 4, 2015 at 3:29 PM Pierre-Yves David <<a href="mailto:pierre-yves.david@ens-lyon.org" target="_blank">pierre-yves.david@ens-lyon.org</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"># HG changeset patch<br>
# User Pierre-Yves David <<a href="mailto:pierre-yves.david@fb.com" target="_blank">pierre-yves.david@fb.com</a>><br>
# Date 1395876090 25200<br>
# Wed Mar 26 16:21:30 2014 -0700<br>
# Node ID dffaf3d15f9587d3f4e00a03cd288108c5cc78fe<br>
# Parent 8aff772b686be3240fc04dc633dff18299ba7151<br>
revset-ancestors: use and iterator instead of a dequeu<br>
<br>
The dequeue was actually just used to be able to pop value one at a time.<br>
Building the dequeue mean we are reading all the input value at once at the<br>
beginning of the evaluation. This defeat the lazyness of revset.<br>
<br>
We replace the deque with iterator usage for the sake of simplicity and<br>
lazyness.<br>
<br>
This provide massive speedup to get the first result if the input set is big<br>
<br>
max(::all())<br>
before) wall 0.001917 comb 0.000000 user 0.000000 sys 0.000000 (best of 1115)<br>
after) wall 0.000107 comb 0.000000 user 0.000000 sys 0.000000 (best of 22222)<br>
<br>
diff --git a/mercurial/revset.py b/mercurial/revset.py<br>
--- a/mercurial/revset.py<br>
+++ b/mercurial/revset.py<br>
@@ -23,27 +23,30 @@ def _revancestors(repo, revs, followfirs<br>
else:<br>
cut = None<br>
cl = repo.changelog<br>
<br>
def iterate():<br>
- revqueue, inputrev = None, None<br>
h = []<br>
<br>
revs.sort(reverse=True)<br>
- revqueue = util.deque(revs)<br>
- if revqueue:<br>
- inputrev = revqueue.popleft()<br>
+ irevs = iter(revs)<br>
+ try:<br>
+ inputrev = irevs.next()<br>
heapq.heappush(h, -inputrev)<br>
+ except StopIteration:<br>
+ irevs, inputrev = None, None<br></blockquote><div><br></div></div></div><div dir="ltr"><div class="gmail_quote"><div>When revs is empty, h will also be empty. So before this patch we could have done "if not revqueue: return" instead, which would have made it clear that inputrev never needs to be initialized to None. Since "return" in a generator can also be written "raise StopIteration", you could simply let the above StopIteration through. Am I correct?</div><div><br></div><div>Since this was similar before and after your patch, perhaps such a simplification should not be baked in to this patch. I do think it would be cleaner to get it in before rather than after, though. But I may very well be missing something here.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"></blockquote></div></div><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
seen = set()<br>
while h:<br>
current = -heapq.heappop(h)<br>
if current not in seen:<br>
- if inputrev and current == inputrev:<br>
- if revqueue:<br>
- inputrev = revqueue.popleft()<br>
+ if irevs is not None and current == inputrev:<br>
+ try:<br></blockquote></div></div><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
+ inputrev = irevs.next()inputrev</blockquote></div></div><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
heapq.heappush(h, -inputrev)<br>
+ except StopIteration:<br>
+ irevs, inputrev = None, None<br></blockquote></div></div><div dir="ltr"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"></blockquote><div><br></div><div>The straight-forwards translation would be "except StopIteration: pass", wouldn't it?</div></div></div><div dir="ltr"><div class="gmail_quote"><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
seen.add(current)<br>
yield current<br>
for parent in cl.parentrevs(current)[:cut]:<br>
if parent != node.nullrev:<br>
heapq.heappush(h, -parent)<br>
_______________________________________________<br>
Mercurial-devel mailing list<br>
<a href="mailto:Mercurial-devel@selenic.com" target="_blank">Mercurial-devel@selenic.com</a><br>
<a href="http://selenic.com/mailman/listinfo/mercurial-devel" target="_blank">http://selenic.com/mailman/listinfo/mercurial-devel</a><br>
</blockquote></div></div></blockquote></div>