This is an archive of the discontinued Mercurial Phabricator instance.

Differential D4322

setdiscovery: don't use dagutil for parent resolution
ClosedPublic

Authored by indygreg on Aug 17 2018, 5:31 PM.

Details

Reviewers
None
Group Reviewers
hg-reviewers
Commits
rHG71d83b315778: setdiscovery: don't use dagutil for parent resolution
Summary

_updatesample()'s one remaining use of revlogdag is for resolving
the parents of a revision.

In 2 cases, we actually resolve parents. In 1, we operate on the
inverted DAG and resolve children.

This commit teaches _updatesample() to receive an argument defining
the function to resolve "parent" revisions. Call sites pass in
changelog.parentrevs() or a wrapper around changelog.children()
accordingly.

The use of children() is semantically correct. But it is quadratic,
since revlog.children() does a range scan over all revisions starting
at its input and effectively calls parentrevs() to build up the list
of children. So calling it repeatedly in a loop is a recipe for
bad performance. I will be implementing something better in a
subsequent commit. I wanted to get the porting off of dagutil done
in a way that was simple and correct.

Like other patches in this series, this change is potentially impacted
but revlogdag's ignorance of filtered revisions. The new code is
filtering aware, since changelog's revs() (used by children() will
skip filtered revisions and therefore hidden children won't appear.
This is potentially backwards incompatible. But no tests fail and
I think this code should respect visibility.

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

indygreg created this revision.Aug 17 2018, 5:31 PM
yuja added a subscriber: yuja.Aug 18 2018, 4:05 AM

+ # TODO this is quadratic
+ parentfn = lambda rev: repo.changelog.children(repo.changelog.node(rev))
+
+ _updatesample(revs, revsroots, sample, parentfn)

Here parentfn() returns a list of nodes, which is wrong. But I took this
since this code is completely rewritten.

Surprisingly, test-setdiscovery.t didn't fail at this revision.

Closed by commit rHG71d83b315778: setdiscovery: don't use dagutil for parent resolution (authored by indygreg). · Explain WhyAug 18 2018, 4:07 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mmercurial/setdiscovery.py (26 lines)

Diff 10443

mercurial/setdiscovery.py

nullrev, nullrev,
) )
from . import ( from . import (
dagutil, dagutil,
error, error,
util, util,
) )
def _updatesample(dag, revs, heads, sample, quicksamplesize=0): def _updatesample(revs, heads, sample, parentfn, quicksamplesize=0):
"""update an existing sample to match the expected size """update an existing sample to match the expected size
The sample is updated with revs exponentially distant from each head of the The sample is updated with revs exponentially distant from each head of the
<revs> set. (H~1, H~2, H~4, H~8, etc). <revs> set. (H~1, H~2, H~4, H~8, etc).
If a target size is specified, the sampling will stop once this size is If a target size is specified, the sampling will stop once this size is
reached. Otherwise sampling will happen until roots of the <revs> set are reached. Otherwise sampling will happen until roots of the <revs> set are
reached. reached.
:dag: a dag object from dagutil
:revs: set of revs we want to discover (if None, assume the whole dag) :revs: set of revs we want to discover (if None, assume the whole dag)
:heads: set of DAG head revs :heads: set of DAG head revs
:sample: a sample to update :sample: a sample to update
:parentfn: a callable to resolve parents for a revision
:quicksamplesize: optional target size of the sample""" :quicksamplesize: optional target size of the sample"""
dist = {} dist = {}
visit = collections.deque(heads) visit = collections.deque(heads)
seen = set() seen = set()
factor = 1 factor = 1
while visit: while visit:
curr = visit.popleft() curr = visit.popleft()
if curr in seen: if curr in seen:
continue continue
d = dist.setdefault(curr, 1) d = dist.setdefault(curr, 1)
if d > factor: if d > factor:
factor *= 2 factor *= 2
if d == factor: if d == factor:
sample.add(curr) sample.add(curr)
if quicksamplesize and (len(sample) >= quicksamplesize): if quicksamplesize and (len(sample) >= quicksamplesize):
return return
seen.add(curr) seen.add(curr)
for p in dag.parents(curr):
if not revs or p in revs: for p in parentfn(curr):
if p != nullrev and (not revs or p in revs):
dist.setdefault(p, d + 1) dist.setdefault(p, d + 1)
visit.append(p) visit.append(p)
def _takequicksample(repo, dag, headrevs, revs, size): def _takequicksample(repo, headrevs, revs, size):
"""takes a quick sample of size <size> """takes a quick sample of size <size>
It is meant for initial sampling and focuses on querying heads and close It is meant for initial sampling and focuses on querying heads and close
ancestors of heads. ancestors of heads.
:dag: a dag object :dag: a dag object
:headrevs: set of head revisions in local DAG to consider :headrevs: set of head revisions in local DAG to consider
:revs: set of revs to discover :revs: set of revs to discover
:size: the maximum size of the sample""" :size: the maximum size of the sample"""
sample = set(repo.revs('heads(%ld)', revs)) sample = set(repo.revs('heads(%ld)', revs))
if len(sample) >= size: if len(sample) >= size:
return _limitsample(sample, size) return _limitsample(sample, size)
_updatesample(dag, None, headrevs, sample, quicksamplesize=size) _updatesample(None, headrevs, sample, repo.changelog.parentrevs,
quicksamplesize=size)
return sample return sample
def _takefullsample(repo, dag, headrevs, revs, size): def _takefullsample(repo, headrevs, revs, size):
sample = set(repo.revs('heads(%ld)', revs)) sample = set(repo.revs('heads(%ld)', revs))
# update from heads # update from heads
revsheads = set(repo.revs('heads(%ld)', revs)) revsheads = set(repo.revs('heads(%ld)', revs))
_updatesample(dag, revs, revsheads, sample) _updatesample(revs, revsheads, sample, repo.changelog.parentrevs)
# update from roots # update from roots
revsroots = set(repo.revs('roots(%ld)', revs)) revsroots = set(repo.revs('roots(%ld)', revs))
_updatesample(dag.inverse(), revs, revsroots, sample)
# TODO this is quadratic
parentfn = lambda rev: repo.changelog.children(repo.changelog.node(rev))
_updatesample(revs, revsroots, sample, parentfn)
assert sample assert sample
sample = _limitsample(sample, size) sample = _limitsample(sample, size)
if len(sample) < size: if len(sample) < size:
more = size - len(sample) more = size - len(sample)
sample.update(random.sample(list(revs - sample), more)) sample.update(random.sample(list(revs - sample), more))
return sample return sample
def _limitsample(sample, desiredlen): def _limitsample(sample, desiredlen):
else: else:
# use even cheaper initial sample # use even cheaper initial sample
ui.debug("taking quick initial sample\n") ui.debug("taking quick initial sample\n")
samplefunc = _takequicksample samplefunc = _takequicksample
targetsize = initialsamplesize targetsize = initialsamplesize
if len(undecided) < targetsize: if len(undecided) < targetsize:
sample = list(undecided) sample = list(undecided)
else: else:
sample = samplefunc(local, dag, ownheads, undecided, targetsize) sample = samplefunc(local, ownheads, undecided, targetsize)
roundtrips += 1 roundtrips += 1
progress.update(roundtrips) progress.update(roundtrips)
ui.debug("query %i; still undecided: %i, sample size is: %i\n" ui.debug("query %i; still undecided: %i, sample size is: %i\n"
% (roundtrips, len(undecided), len(sample))) % (roundtrips, len(undecided), len(sample)))
# indices between sample and externalized version must match # indices between sample and externalized version must match
sample = list(sample) sample = list(sample)