Patchwork [2,of,8] discovery: introduce a partialdiscovery object

login
register
mail settings
Submitter Boris Feld
Date Dec. 31, 2018, 5:35 p.m.
Message ID <db1f1838096a110cc473.1546277749@Laptop-Boris.lan>
Download mbox | patch
Permalink /patch/37408/
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 1545963274 -3600
#      Fri Dec 28 03:14:34 2018 +0100
# Node ID db1f1838096a110cc4735aeaffac0e65bde37b50
# Parent  f94ade1a2714d498d05f5b7714f4b869224c0596
# EXP-Topic discovery-refactor
# Available At https://bitbucket.org/octobus/mercurial-devel/
#              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r db1f1838096a
discovery: introduce a partialdiscovery object

This object will ultimately gather the data about common, undecided and
missing revs in a single place and deal with most graph related computations.

The goal is both to clarify the algorithm and to help provides a simple and
clear API that can be reimplemented in Rust.

For now, we only moved the `common` set in the object. In this commit, some
direct access to the "private" `disco._common` attribute persist. They have
not been removed yet because we won't need to expose a full API identical to
`incrementalmissingancestors` and it seems simpler to access the attribute
directly until the replacement is in place.
Pulkit Goyal - Jan. 4, 2019, 4:01 p.m.
On Mon, Dec 31, 2018 at 11:13 PM Boris Feld <boris.feld@octobus.net> wrote:

> # HG changeset patch
> # User Boris Feld <boris.feld@octobus.net>
> # Date 1545963274 -3600
> #      Fri Dec 28 03:14:34 2018 +0100
> # Node ID db1f1838096a110cc4735aeaffac0e65bde37b50
> # Parent  f94ade1a2714d498d05f5b7714f4b869224c0596
> # EXP-Topic discovery-refactor
> # Available At https://bitbucket.org/octobus/mercurial-devel/
> #              hg pull https://bitbucket.org/octobus/mercurial-devel/ -r
> db1f1838096a
> discovery: introduce a partialdiscovery object
>
> This object will ultimately gather the data about common, undecided and
> missing revs in a single place and deal with most graph related
> computations.
>
> The goal is both to clarify the algorithm and to help provides a simple and
> clear API that can be reimplemented in Rust.
>
> For now, we only moved the `common` set in the object. In this commit, some
> direct access to the "private" `disco._common` attribute persist. They have
> not been removed yet because we won't need to expose a full API identical
> to
> `incrementalmissingancestors` and it seems simpler to access the attribute
> directly until the replacement is in place.
>
> diff --git a/mercurial/setdiscovery.py b/mercurial/setdiscovery.py
> --- a/mercurial/setdiscovery.py
> +++ b/mercurial/setdiscovery.py
> @@ -161,6 +161,28 @@ def _limitsample(sample, desiredlen):
>          sample = set(random.sample(sample, desiredlen))
>      return sample
>
> +class partialdiscovery(object):
> +    """an object representing ongoing discovery
> +
> +    Feed with data from the remote repository, this object keep track of
> the
> +    current set of changeset in various states:
> +
> +    - common: own nodes I know we both know
> +    """
>

"own nodes I know we both know" -> this is not a helpful documentation.
Boris Feld - Jan. 4, 2019, 5:27 p.m.
On 04/01/2019 17:01, Pulkit Goyal wrote:
>
>
> On Mon, Dec 31, 2018 at 11:13 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 1545963274 -3600
>     #      Fri Dec 28 03:14:34 2018 +0100
>     # Node ID db1f1838096a110cc4735aeaffac0e65bde37b50
>     # Parent  f94ade1a2714d498d05f5b7714f4b869224c0596
>     # EXP-Topic discovery-refactor
>     # Available At https://bitbucket.org/octobus/mercurial-devel/
>     #              hg pull
>     https://bitbucket.org/octobus/mercurial-devel/ -r db1f1838096a
>     discovery: introduce a partialdiscovery object
>
>     This object will ultimately gather the data about common,
>     undecided and
>     missing revs in a single place and deal with most graph related
>     computations.
>
>     The goal is both to clarify the algorithm and to help provides a
>     simple and
>     clear API that can be reimplemented in Rust.
>
>     For now, we only moved the `common` set in the object. In this
>     commit, some
>     direct access to the "private" `disco._common` attribute persist.
>     They have
>     not been removed yet because we won't need to expose a full API
>     identical to
>     `incrementalmissingancestors` and it seems simpler to access the
>     attribute
>     directly until the replacement is in place.
>
>     diff --git a/mercurial/setdiscovery.py b/mercurial/setdiscovery.py
>     --- a/mercurial/setdiscovery.py
>     +++ b/mercurial/setdiscovery.py
>     @@ -161,6 +161,28 @@ def _limitsample(sample, desiredlen):
>              sample = set(random.sample(sample, desiredlen))
>          return sample
>
>     +class partialdiscovery(object):
>     +    """an object representing ongoing discovery
>     +
>     +    Feed with data from the remote repository, this object keep
>     track of the
>     +    current set of changeset in various states:
>     +
>     +    - common: own nodes I know we both know
>     +    """
>
>
> "own nodes I know we both know" -> this is not a helpful documentation.
I'm reusing the very same comment that used to be in the code. Can we
make a documentation pass as a followup?
>
> _______________________________________________
> 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
@@ -161,6 +161,28 @@  def _limitsample(sample, desiredlen):
         sample = set(random.sample(sample, desiredlen))
     return sample
 
+class partialdiscovery(object):
+    """an object representing ongoing discovery
+
+    Feed with data from the remote repository, this object keep track of the
+    current set of changeset in various states:
+
+    - common: own nodes I know we both know
+    """
+
+    def __init__(self, repo):
+        self._repo = repo
+        self._common = repo.changelog.incrementalmissingrevs()
+
+    def addcommons(self, commons):
+        """registrer nodes known as common"""
+        self._common.addbases(commons)
+
+    def hasinfo(self):
+        """return True is we have any clue about the remote state"""
+        return self._common.hasbases()
+
+
 def findcommonheads(ui, local, remote,
                     initialsamplesize=100,
                     fullsamplesize=200,
@@ -227,14 +249,14 @@  def findcommonheads(ui, local, remote,
 
     # full blown discovery
 
-    # own nodes I know we both know
+    disco = partialdiscovery(local)
     # treat remote heads (and maybe own heads) as a first implicit sample
     # response
-    common = cl.incrementalmissingrevs(srvheads)
+    disco.addcommons(srvheads)
     commoninsample = set(n for i, n in enumerate(sample) if yesno[i])
-    common.addbases(commoninsample)
+    disco.addcommons(commoninsample)
     # own nodes where I don't know if remote knows them
-    undecided = set(common.missingancestors(ownheads))
+    undecided = set(disco._common.missingancestors(ownheads))
     # own nodes I know remote lacks
     missing = set()
 
@@ -256,7 +278,7 @@  def findcommonheads(ui, local, remote,
         if not undecided:
             break
 
-        if full or common.hasbases():
+        if full or disco.hasinfo():
             if full:
                 ui.note(_("sampling from both directions\n"))
             else:
@@ -286,13 +308,13 @@  def findcommonheads(ui, local, remote,
 
         if sample:
             commoninsample = set(n for i, n in enumerate(sample) if yesno[i])
-            common.addbases(commoninsample)
-            common.removeancestorsfrom(undecided)
+            disco.addcommons(commoninsample)
+            disco._common.removeancestorsfrom(undecided)
 
     # heads(common) == heads(common.bases) since common represents common.bases
     # and all its ancestors
     # The presence of nullrev will confuse heads(). So filter it out.
-    result = set(local.revs('heads(%ld)', common.bases - {nullrev}))
+    result = set(local.revs('heads(%ld)', disco._common.bases - {nullrev}))
     elapsed = util.timer() - start
     progress.complete()
     ui.debug("%d total queries in %.4fs\n" % (roundtrips, elapsed))