This is an archive of the discontinued Mercurial Phabricator instance.

Differential D2643

setdiscovery: back out changeset 5cfdf6137af8 (issue5809)
ClosedPublic

Authored by martinvonz on Mar 4 2018, 11:58 AM.

Details

Reviewers
indygreg
Group Reviewers
hg-reviewers
Commits
rHG613954a17a25: setdiscovery: back out changeset 5cfdf6137af8 (issue5809)
Summary

As explained in the bug report, this commit caused a performance
regression. The problem occurs when the local repo has very many
heads. Before 5cfdf6137af8, we used to get the remote's list of heads
and if these heads mostly overlapped with the local repo's heads, we
would mark these common heads as common, which would greatly reduce
the size of the set of undecided nodes.

Note that a similar problem existed before 5cfdf6137af8: If the local
repo had very many heads and the server just had a few (or many heads
from a disjoint set), we would do the same kind of slow discovery as
we would with 5cfdf6137af8 in the case where local and remote repos
share a large set of common nodes.

For now, we just back out 5cfdf6137af8. We should improve the
discovery in the "local has many heads, remote has few heads" case,
but let's do that after backing this out.

Diff Detail

Repository
rHG Mercurial
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

martinvonz created this revision.Mar 4 2018, 11:58 AM
indygreg accepted this revision.Mar 4 2018, 2:18 PM
This revision is now accepted and ready to land.Mar 4 2018, 2:18 PM
Closed by commit rHG613954a17a25: setdiscovery: back out changeset 5cfdf6137af8 (issue5809) (authored by martinvonz). · Explain WhyMar 4 2018, 2:29 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/discovery.py (2 lines)
Mmercurial/setdiscovery.py (16 lines)

Diff 6593

mercurial/discovery.py

if not remote.capable('getbundle'): if not remote.capable('getbundle'):
return treediscovery.findcommonincoming(repo, remote, heads, force) return treediscovery.findcommonincoming(repo, remote, heads, force)
if heads: if heads:
knownnode = repo.changelog.hasnode # no nodemap until it is filtered knownnode = repo.changelog.hasnode # no nodemap until it is filtered
if all(knownnode(h) for h in heads): if all(knownnode(h) for h in heads):
return (heads, False, heads) return (heads, False, heads)
res = setdiscovery.findcommonheads(repo.ui, repo, remote, heads, res = setdiscovery.findcommonheads(repo.ui, repo, remote,
abortwhenunrelated=not force, abortwhenunrelated=not force,
ancestorsof=ancestorsof) ancestorsof=ancestorsof)
common, anyinc, srvheads = res common, anyinc, srvheads = res
return (list(common), anyinc, heads or list(srvheads)) return (list(common), anyinc, heads or list(srvheads))
class outgoing(object): class outgoing(object):
'''Represents the set of nodes present in a local repo but not in a '''Represents the set of nodes present in a local repo but not in a
(possibly) remote one. (possibly) remote one.

mercurial/setdiscovery.py

return sample return sample
def _limitsample(sample, desiredlen): def _limitsample(sample, desiredlen):
"""return a random subset of sample of at most desiredlen item""" """return a random subset of sample of at most desiredlen item"""
if len(sample) > desiredlen: if len(sample) > desiredlen:
sample = set(random.sample(sample, desiredlen)) sample = set(random.sample(sample, desiredlen))
return sample return sample
def findcommonheads(ui, local, remote, heads=None, def findcommonheads(ui, local, remote,
initialsamplesize=100, initialsamplesize=100,
fullsamplesize=200, fullsamplesize=200,
abortwhenunrelated=True, abortwhenunrelated=True,
ancestorsof=None): ancestorsof=None):
'''Return a tuple (common, anyincoming, remoteheads) used to identify '''Return a tuple (common, anyincoming, remoteheads) used to identify
missing nodes from or in remote. missing nodes from or in remote.
''' '''
start = util.timer() start = util.timer()
roundtrips = 0 roundtrips = 0
cl = local.changelog cl = local.changelog
localsubset = None localsubset = None
if ancestorsof is not None: if ancestorsof is not None:
rev = local.changelog.rev rev = local.changelog.rev
localsubset = [rev(n) for n in ancestorsof] localsubset = [rev(n) for n in ancestorsof]
dag = dagutil.revlogdag(cl, localsubset=localsubset) dag = dagutil.revlogdag(cl, localsubset=localsubset)
# early exit if we know all the specified remote heads already # early exit if we know all the specified remote heads already
ui.debug("query 1; heads\n") ui.debug("query 1; heads\n")
roundtrips += 1 roundtrips += 1
ownheads = dag.heads() ownheads = dag.heads()
sample = _limitsample(ownheads, initialsamplesize) sample = _limitsample(ownheads, initialsamplesize)
# indices between sample and externalized version must match # indices between sample and externalized version must match
sample = list(sample) sample = list(sample)
if heads:
srvheadhashes = heads
yesno = remote.known(dag.externalizeall(sample))
else:
batch = remote.iterbatch() batch = remote.iterbatch()
batch.heads() batch.heads()
batch.known(dag.externalizeall(sample)) batch.known(dag.externalizeall(sample))
batch.submit() batch.submit()
srvheadhashes, yesno = batch.results() srvheadhashes, yesno = batch.results()
if cl.tip() == nullid: if cl.tip() == nullid:
if srvheadhashes != [nullid]: if srvheadhashes != [nullid]:
return [nullid], True, srvheadhashes return [nullid], True, srvheadhashes
return [nullid], False, [] return [nullid], False, []
# start actual discovery (we note this before the next "if" for # start actual discovery (we note this before the next "if" for
# compatibility reasons) # compatibility reasons)