Patchwork [V2] discovery: be more conservative when adjusting the sample size

login
register
mail settings
Submitter Pierre-Yves David
Date June 6, 2019, 8:52 a.m.
Message ID <84886efab235050be22d.1559811142@nodosa.octopoid.net>
Download mbox | patch
Permalink /patch/40334/
State Accepted
Headers show

Comments

Pierre-Yves David - June 6, 2019, 8:52 a.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@octobus.net>
# Date 1559726605 -7200
#      Wed Jun 05 11:23:25 2019 +0200
# Node ID 84886efab235050be22d3fd737226c6473004f24
# Parent  12793787439538411013edffe0f9b98762d38a37
# EXP-Topic discovery-large-undecided
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 84886efab235
discovery: be more conservative when adjusting the sample size

Since 5b34972a0094, the discovery will increase the sample size when it detect a
"complex" undecided set. However this detection focussed on the number of roots
only, this could regress discovery performance when the undecided set has many
roots that eventually get merged into a few heads.

To prevent such misbehavior, we adjust the logic to take in account both heads
and roots. The sample size will be increased only if both are especially large.

Performance testing on the same case as 5b34972a0094, does not show a
significant difference.
Yuya Nishihara - June 8, 2019, 1:36 a.m.
On Thu, 06 Jun 2019 10:52:22 +0200, Pierre-Yves David wrote:
> # HG changeset patch
> # User Pierre-Yves David <pierre-yves.david@octobus.net>
> # Date 1559726605 -7200
> #      Wed Jun 05 11:23:25 2019 +0200
> # Node ID 84886efab235050be22d3fd737226c6473004f24
> # Parent  12793787439538411013edffe0f9b98762d38a37
> # EXP-Topic discovery-large-undecided
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r 84886efab235
> discovery: be more conservative when adjusting the sample size

Queued with minor tweaks, thanks.

> diff --git a/mercurial/setdiscovery.py b/mercurial/setdiscovery.py
> --- a/mercurial/setdiscovery.py
> +++ b/mercurial/setdiscovery.py
> @@ -241,14 +241,18 @@ class partialdiscovery(object):
>          _updatesample(revs, revsheads, sample, parentrevs)
>  
>          # update from roots
> -        revsroots = set(repo.revs('roots(%ld)', revs))
> -        if not self._respectsize:
> -            size = max(size, len(revsroots))
>  
>          childrenrevs = self._childrengetter()
>  
> +        revsroots = set(repo.revs('roots(%ld)', revs))
>          _updatesample(revs, revsroots, sample, childrenrevs)
>          assert sample
> +
> +        if not self._respectsize:
> +            nbroots = len(revsroots)
> +            nbheads = len(revsheads)
> +            size = max(size, min(nbroots, nbheads))

Inlined these variables since len(xxx) seems more readable than nbxxx.

Patch

diff --git a/mercurial/setdiscovery.py b/mercurial/setdiscovery.py
--- a/mercurial/setdiscovery.py
+++ b/mercurial/setdiscovery.py
@@ -241,14 +241,18 @@  class partialdiscovery(object):
         _updatesample(revs, revsheads, sample, parentrevs)
 
         # update from roots
-        revsroots = set(repo.revs('roots(%ld)', revs))
-        if not self._respectsize:
-            size = max(size, len(revsroots))
 
         childrenrevs = self._childrengetter()
 
+        revsroots = set(repo.revs('roots(%ld)', revs))
         _updatesample(revs, revsroots, sample, childrenrevs)
         assert sample
+
+        if not self._respectsize:
+            nbroots = len(revsroots)
+            nbheads = len(revsheads)
+            size = max(size, min(nbroots, nbheads))
+
         sample = _limitsample(sample, size)
         if len(sample) < size:
             more = size - len(sample)