Patchwork [1,of,8] discovery: move handling of sampling special case inside sampling function

login
register
mail settings
Submitter Boris Feld
Date Dec. 31, 2018, 5:35 p.m.
Message ID <f94ade1a2714d498d05f.1546277748@Laptop-Boris.lan>
Download mbox | patch
Permalink /patch/37405/
State Superseded
Headers show

Comments

Boris Feld - Dec. 31, 2018, 5:35 p.m.
# HG changeset patch
# User Boris Feld <boris.feld@octobus.net>
# Date 1544785275 -3600
#      Fri Dec 14 12:01:15 2018 +0100
# Node ID f94ade1a2714d498d05f5b7714f4b869224c0596
# Parent  9bfbb9fc58711308708003846f8beb63ac21b9d0
# EXP-Topic discovery-refactor
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r f94ade1a2714
discovery: move handling of sampling special case inside sampling function

The handling of cases where the number of revisions to sample is smaller than
the sample size can be moved with the sample function themselves. This
simplifies main logic, preparing a coming refactoring.
Pulkit Goyal - Jan. 4, 2019, 3:47 p.m.
On Mon, Dec 31, 2018 at 11:08 PM Boris Feld <boris.feld@octobus.net> wrote:

> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1544785275 -3600
> #      Fri Dec 14 12:01:15 2018 +0100
> # Node ID f94ade1a2714d498d05f5b7714f4b869224c0596
> # Parent  9bfbb9fc58711308708003846f8beb63ac21b9d0
> # EXP-Topic discovery-refactor
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r
> f94ade1a2714
> discovery: move handling of sampling special case inside sampling function
>
> The handling of cases where the number of revisions to sample is smaller
> than
> the sample size can be moved with the sample function themselves. This
> simplifies main logic, preparing a coming refactoring.
>
> diff --git a/mercurial/setdiscovery.py b/mercurial/setdiscovery.py
> --- a/mercurial/setdiscovery.py
> +++ b/mercurial/setdiscovery.py
> @@ -102,6 +102,8 @@ def _takequicksample(repo, headrevs, rev
>      :headrevs: set of head revisions in local DAG to consider
>      :revs: set of revs to discover
>      :size: the maximum size of the sample"""
> +    if len(revs) <= size:
> +        return list(revs)
>

Should not this be '<' instead of '<='?

>      sample = set(repo.revs('heads(%ld)', revs))
>
>      if len(sample) >= size:
> @@ -112,6 +114,8 @@ def _takequicksample(repo, headrevs, rev
>      return sample
>
>  def _takefullsample(repo, headrevs, revs, size):
> +    if len(revs) <= size:
> +        return list(revs)
>

Same.

>      sample = set(repo.revs('heads(%ld)', revs))
>
>      # update from heads
> @@ -264,10 +268,7 @@ def findcommonheads(ui, local, remote,
>              ui.debug("taking quick initial sample\n")
>              samplefunc = _takequicksample
>              targetsize = initialsamplesize
> -        if len(undecided) < targetsize:
>

This is just '<' here.

> -            sample = list(undecided)
> -        else:
> -            sample = samplefunc(local, ownheads, undecided, targetsize)
> +        sample = samplefunc(local, ownheads, undecided, targetsize)
>
>          roundtrips += 1
>          progress.update(roundtrips)
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
Boris Feld - Jan. 4, 2019, 5:26 p.m.
On 04/01/2019 16:47, Pulkit Goyal wrote:
>
>
> On Mon, Dec 31, 2018 at 11:08 PM Boris Feld <boris.feld@octobus.net
> <mailto:boris.feld@octobus.net>> wrote:
>
>     # HG changeset patch
>     # User Boris Feld <boris.feld@octobus.net
>     <mailto:boris.feld@octobus.net>>
>     # Date 1544785275 -3600
>     #      Fri Dec 14 12:01:15 2018 +0100
>     # Node ID f94ade1a2714d498d05f5b7714f4b869224c0596
>     # Parent  9bfbb9fc58711308708003846f8beb63ac21b9d0
>     # EXP-Topic discovery-refactor
>     # Available At https://bitbucket.org/octobus/mercurial-devel/
>     #              hg pull
>     https://bitbucket.org/octobus/mercurial-devel/ -r f94ade1a2714
>     discovery: move handling of sampling special case inside sampling
>     function
>
>     The handling of cases where the number of revisions to sample is
>     smaller than
>     the sample size can be moved with the sample function themselves. This
>     simplifies main logic, preparing a coming refactoring.
>
>     diff --git a/mercurial/setdiscovery.py b/mercurial/setdiscovery.py
>     --- a/mercurial/setdiscovery.py
>     +++ b/mercurial/setdiscovery.py
>     @@ -102,6 +102,8 @@ def _takequicksample(repo, headrevs, rev
>          :headrevs: set of head revisions in local DAG to consider
>          :revs: set of revs to discover
>          :size: the maximum size of the sample"""
>     +    if len(revs) <= size:
>     +        return list(revs)
>
>
> Should not this be '<' instead of '<='?

The initial code was "buggy". "size" is the target size so entry up to
size are okay.

Do you want the change to be introduced explicitly in an earlier changeset?
>
>          sample = set(repo.revs('heads(%ld)', revs))
>
>          if len(sample) >= size:
>     @@ -112,6 +114,8 @@ def _takequicksample(repo, headrevs, rev
>          return sample
>
>      def _takefullsample(repo, headrevs, revs, size):
>     +    if len(revs) <= size:
>     +        return list(revs)
>
>
> Same.
>
>          sample = set(repo.revs('heads(%ld)', revs))
>
>          # update from heads
>     @@ -264,10 +268,7 @@ def findcommonheads(ui, local, remote,
>                  ui.debug("taking quick initial sample\n")
>                  samplefunc = _takequicksample
>                  targetsize = initialsamplesize
>     -        if len(undecided) < targetsize:
>
>
> This is just '<' here.
>
>     -            sample = list(undecided)
>     -        else:
>     -            sample = samplefunc(local, ownheads, undecided,
>     targetsize)
>     +        sample = samplefunc(local, ownheads, undecided, targetsize)
>
>              roundtrips += 1
>              progress.update(roundtrips)
>     _______________________________________________
>     Mercurial-devel mailing list
>     Mercurial-devel@mercurial-scm.org
>     <mailto:Mercurial-devel@mercurial-scm.org>
>     https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/setdiscovery.py b/mercurial/setdiscovery.py
--- a/mercurial/setdiscovery.py
+++ b/mercurial/setdiscovery.py
@@ -102,6 +102,8 @@  def _takequicksample(repo, headrevs, rev
     :headrevs: set of head revisions in local DAG to consider
     :revs: set of revs to discover
     :size: the maximum size of the sample"""
+    if len(revs) <= size:
+        return list(revs)
     sample = set(repo.revs('heads(%ld)', revs))
 
     if len(sample) >= size:
@@ -112,6 +114,8 @@  def _takequicksample(repo, headrevs, rev
     return sample
 
 def _takefullsample(repo, headrevs, revs, size):
+    if len(revs) <= size:
+        return list(revs)
     sample = set(repo.revs('heads(%ld)', revs))
 
     # update from heads
@@ -264,10 +268,7 @@  def findcommonheads(ui, local, remote,
             ui.debug("taking quick initial sample\n")
             samplefunc = _takequicksample
             targetsize = initialsamplesize
-        if len(undecided) < targetsize:
-            sample = list(undecided)
-        else:
-            sample = samplefunc(local, ownheads, undecided, targetsize)
+        sample = samplefunc(local, ownheads, undecided, targetsize)
 
         roundtrips += 1
         progress.update(roundtrips)