[PATCH 1 of 5] revset: adds onlyroots argument to revsbetween

Laurent Charignon lcharignon at fb.com
Fri Aug 7 04:25:41 CDT 2015


On Aug 7, 2015, at 12:34 AM, Pierre-Yves David <pierre-yves.david at ens-lyon.org<mailto:pierre-yves.david at ens-lyon.org>> wrote:



On 08/06/2015 11:10 PM, Laurent Charignon wrote:
# HG changeset patch
# User Laurent Charignon <lcharignon at fb.com<mailto:lcharignon at fb.com>>
# Date 1434770334 25200
#      Fri Jun 19 20:18:54 2015 -0700
# Branch stable
# Node ID dd83ca8e712cec7331a8418600da7c5a8b8cfed0
# Parent  79f0cb97d7537a7c2948f8f9b0a89148825a3a1d
revset: adds onlyroots argument to revsbetween

This patch is part of a series of patches to speed up the computation of
revset.revsbetween by introducing a C implementation. The main motivation is to
speed up smartlog on big repositories. At the end of the series, on our big
repositories the computation of revsbetween is 10-50x faster and smartlog on is
2x-5x faster.

This patch adds a new argument to revsbetween called onlyroots. This allows us
to compute grandparent with revsbetween and remove the code of grandparent in
the next patch.

That patch looks good to me.

I've some reservation about the name, revbetween(onlyroots=False) seems a bit weird. I would probably invert the logic in the name. Something like:

 reachedroots(includepath=True)

Yes, includepath seems like a better name, I will send a V2

However, ss we can change it later, I do not think we should delay this patch on this.

diff --git a/mercurial/revset.py b/mercurial/revset.py
--- a/mercurial/revset.py
+++ b/mercurial/revset.py
@@ -78,7 +78,7 @@

     return generatorset(iterate(), iterasc=True)

-def _revsbetween(repo, roots, heads):
+def _revsbetween(repo, roots, heads, onlyroots=False):
     """Return all paths between roots and heads, inclusive of both endpoint
     sets."""
     if not roots:
@@ -101,6 +101,8 @@
         rev = nextvisit()
         if rev in roots:
             reached(rev)
+            if onlyroots:
+                continue

Did you benchmark the performance impact of this addition?

We can mitigate the perf impact by duping the code but I am not sure that it is worth it because it seems to come pretty close:


$ echo ".~(200)::." | python contrib/revsetbenchmarks.py ". + .^"
Revsets to benchmark
----------------------------
0) .~(200)::.
----------------------------

----------------------------
 26789:dd83ca8e712c: revset: adds onlyroots argument to revsbetween
----------------------------
   plain    min      max      first    last     reverse  rev..rst rev..ast sort     sor..rst sor..ast
0) 0.004194 0.004192 0.004503 0.004102 0.004112 0.004171 0.004288 0.004307 0.004297 0.004174 0.004438
----------------------------
----------------------------
 26726:79f0cb97d753: Added signature for changeset 21aa1c313b05
----------------------------
   plain    min      max      first    last     reverse  rev..rst rev..ast sort     sor..rst sor..ast
0) 0.004219 0.004430 0.004205 0.004094 0.004247 0.004120 0.004262 0.004165 0.004276 0.004338 0.004376
----------------------------


Result by revset
================

Revision:
 26789:dd83ca8e712c: revset: adds onlyroots argument to revsbetween
 26726:79f0cb97d753: Added signature for changeset 21aa1c313b05


revset #0: .~(200)::.
   plain         min           max           first         last          reverse       rev..rst      rev..ast      sort          sor..rst      sor..ast
0) 0.004194      0.004192      0.004503      0.004102      0.004112      0.004171      0.004288      0.004307      0.004297      0.004174      0.004438
1) 0.004219      0.004430 105% 0.004205  93% 0.004094      0.004247      0.004120      0.004262      0.004165      0.004276      0.004338      0.004376



--
Pierre-Yves David

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20150807/39720861/attachment.html>


More information about the Mercurial-devel mailing list