mercurial/revset.py
1603	Isn't "getphase(repo,rev) >= keepphase" more natural? You'd then replace phasenamemap by something like minphase = { '_notpublic': draft, 'draft': draft, 'secret': secret, }

mercurial/revset.py
1603	Good point. Thanks!

tests/test-revset.t
4344	Need to change to just `P5`.

mercurial/dagop.py
93	Could use `revs.filter` instead.

mercurial/revsetlang.py
432	FWIW, I wrote placeholder matcher once, which could be used as follows. m = _matchtree('_() & ::_', ('and', ta, tb)) if m and getsymbol(m[1]) in _fastphases: ... https://bitbucket.org/yuja/hg-work/commits/6c0ae642d86a188af8e940e430522e915f478b25 This is outdated and needs modification, but do you like it?

mercurial/revsetlang.py
432	Yes! That's much shorter (I had to copy the trees from debugshell) and more robust (no fragile `len(x) >= 3` handling). For performance concerns, I think it's fine. Practically, revset parsing is hardly a bottleneck. With chg, `_treecache` could be pre-populated.

mercurial/revsetlang.py
432	I'll rebase my patches then. I'm sure that isn't trivial because I introduced the order flag.

mercurial/revsetlang.py
432	Maybe `order` could be removed from AST since they can be inferred during tree traversal? I have a draft D451 that verifies the idea.

mercurial/revset.py
1607	`revs & phasecache.getrevset(repo, [phasename])` might be slightly faster, but I don't think it would make any noticeable difference.
mercurial/revsetlang.py
416	Perhaps `::x & phasename()` can be optimized, too.

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	1056		Aug 18 2017, 12:50 AM	★	★
Diff 2	1057		Aug 18 2017, 12:56 AM	★	★
Diff 3	1058		Aug 18 2017, 1:25 AM	★	★
Diff 4	1059		Aug 18 2017, 1:44 AM	★	★
Diff 5	1071		Aug 18 2017, 10:50 AM	★	★
Diff 6	1074		Aug 18 2017, 1:18 PM	★	★
Diff 7	1075		Aug 18 2017, 1:27 PM	★	★
Diff 8	1570		Sep 1 2017, 9:24 PM	★	★
Diff 9	1571		Sep 1 2017, 9:28 PM	★	★
Diff 10	1591	rHG68afb88b51bb626cd25440e2df812f2306463b55	Aug 28 2017, 5:49 PM	★	★

Diff 1570

mercurial/dagop.py

	lastrev = currev			lastrev = currev
	yield currev			yield currev
	pdepth = curdepth + 1			pdepth = curdepth + 1
	if foundnew and pdepth < stopdepth:			if foundnew and pdepth < stopdepth:
	for prev in pfunc(currev):			for prev in pfunc(currev):
	if prev != node.nullrev:			if prev != node.nullrev:
	heapq.heappush(pendingheap, (heapsign * prev, pdepth))			heapq.heappush(pendingheap, (heapsign * prev, pdepth))

	def _genrevancestors(repo, revs, followfirst, startdepth, stopdepth):			def _genrevancestors(repo, revs, followfirst, startdepth, stopdepth, cutfunc):
	if followfirst:			if followfirst:
	cut = 1			cut = 1
	else:			else:
	cut = None			cut = None
	cl = repo.changelog			cl = repo.changelog
	def pfunc(rev):			def plainpfunc(rev):
	try:			try:
	return cl.parentrevs(rev)[:cut]			return cl.parentrevs(rev)[:cut]
	except error.WdirUnsupported:			except error.WdirUnsupported:
	return (pctx.rev() for pctx in repo[rev].parents()[:cut])			return (pctx.rev() for pctx in repo[rev].parents()[:cut])
				if cutfunc is None:
				pfunc = plainpfunc
				else:
				pfunc = lambda rev: [r for r in plainpfunc(rev) if not cutfunc(r)]
				revs = revs.filter(lambda rev: not cutfunc(rev))
				quarkAuthorUnsubmitted Done Could use `revs.filter` instead. quark: Could use `revs.filter` instead.
	return _walkrevtree(pfunc, revs, startdepth, stopdepth, reverse=True)			return _walkrevtree(pfunc, revs, startdepth, stopdepth, reverse=True)

	def revancestors(repo, revs, followfirst, startdepth=None, stopdepth=None):			def revancestors(repo, revs, followfirst=False, startdepth=None,
				stopdepth=None, cutfunc=None):
	"""Like revlog.ancestors(), but supports additional options, includes			"""Like revlog.ancestors(), but supports additional options, includes
	the given revs themselves, and returns a smartset			the given revs themselves, and returns a smartset

	Scan ends at the stopdepth (exlusive) if specified. Revisions found			Scan ends at the stopdepth (exlusive) if specified. Revisions found
	earlier than the startdepth are omitted.			earlier than the startdepth are omitted.

				If cutfunc is provided, it will be used to cut the traversal of the DAG.
				When cutfunc(X) returns False, the DAG traversal stops - revision X and
				X's ancestors in the traversal path will be skipped. This could be an
				optimization sometimes.

				Note: if Y is an ancestor of X, cutfunc(X) returning True does not
				necessarily mean Y will also be cut. Usually cutfunc(Y) also wants to
				return True in this case. For example,

				D # revancestors(repo, D, cutfunc=lambda rev: rev == B)
				\|\ # will include "A", because the path D -> C -> A was not cut.
				B C # If "B" gets cut, "A" might want to be cut too.
				\|/
				A
	"""			"""
	gen = _genrevancestors(repo, revs, followfirst, startdepth, stopdepth)			gen = _genrevancestors(repo, revs, followfirst, startdepth, stopdepth,
				cutfunc)
	return generatorset(gen, iterasc=False)			return generatorset(gen, iterasc=False)

	def _genrevdescendants(repo, revs, followfirst):			def _genrevdescendants(repo, revs, followfirst):
	if followfirst:			if followfirst:
	cut = 1			cut = 1
	else:			else:
	cut = None			cut = None

mercurial/revset.py

	return baseset()			return baseset()

	# for internal use			# for internal use
	@predicate('_notpublic', safe=True)			@predicate('_notpublic', safe=True)
	def _notpublic(repo, subset, x):			def _notpublic(repo, subset, x):
	getargs(x, 0, 0, "_notpublic takes no arguments")			getargs(x, 0, 0, "_notpublic takes no arguments")
	return _phase(repo, subset, phases.draft, phases.secret)			return _phase(repo, subset, phases.draft, phases.secret)

				@predicate('_phaseandancestors(phasename, set)', safe=True)
				def _phaseandancestors(repo, subset, x):
				# equivalent to (phasename() & ancestors(set)) but more efficient
				# phasename could be one of 'draft', 'secret', or '_notpublic'
				args = getargs(x, 2, 2, _("_phaseandancestors requires two arguments"))
				phasename = getsymbol(args[0])
				s = getset(repo, fullreposet(repo), args[1])

				draft = phases.draft
				secret = phases.secret
				phasenamemap = {
				'_notpublic': draft,
				'draft': draft, # follow secret's ancestors
				'secret': secret,
				}
				if phasename not in phasenamemap:
				raise error.ParseError(_('%r is not a valid phasename') % phasename)

				minimalphase = phasenamemap[phasename]
				getphase = repo._phasecache.phase

				def cutfunc(rev):
				return getphase(repo, rev) < minimalphase

				martinvonzUnsubmitted Done Isn't "getphase(repo,rev) >= keepphase" more natural? You'd then replace phasenamemap by something like minphase = { '_notpublic': draft, 'draft': draft, 'secret': secret, } martinvonz: Isn't "getphase(repo,rev) >= keepphase" more natural? You'd then replace phasenamemap by…
				quarkAuthorUnsubmitted Not Done Good point. Thanks! quark: Good point. Thanks!
				revs = dagop.revancestors(repo, s, cutfunc=cutfunc)

				if phasename == 'draft': # need to remove secret changesets
				revs = revs.filter(lambda r: getphase(repo, r) == draft)
				yujaUnsubmitted Not Done `revs & phasecache.getrevset(repo, [phasename])` might be slightly faster, but I don't think it would make any noticeable difference. yuja: `revs & phasecache.getrevset(repo, [phasename])` might be slightly faster, but I don't think it…
				return subset & revs

	@predicate('public()', safe=True)			@predicate('public()', safe=True)
	def public(repo, subset, x):			def public(repo, subset, x):
	"""Changeset in public phase."""			"""Changeset in public phase."""
	# i18n: "public" is a keyword			# i18n: "public" is a keyword
	getargs(x, 0, 0, _("public takes no arguments"))			getargs(x, 0, 0, _("public takes no arguments"))
	phase = repo._phasecache.phase			phase = repo._phasecache.phase
	target = phases.public			target = phases.public
	condition = lambda r: phase(repo, r) == target			condition = lambda r: phase(repo, r) == target

mercurial/revsetlang.py

	"""			"""
	pattern = _cachedtree(patspec)			pattern = _cachedtree(patspec)
	return parser.matchtree(pattern, tree, ('symbol', '_'),			return parser.matchtree(pattern, tree, ('symbol', '_'),
	{'keyvalue', 'list'})			{'keyvalue', 'list'})

	def _matchonly(revs, bases):			def _matchonly(revs, bases):
	return _match('ancestors(_) and not ancestors(_)', ('and', revs, bases))			return _match('ancestors(_) and not ancestors(_)', ('and', revs, bases))

				def _matchtree(tree, pattern):
				"""like re.match, return matched patterns or None if not matched

				A fixed string matches a fixed string. A lambda matches and captures things
				if it returns True. A tuple will trigger a recursive match on its elements.
				Return a list of captured subtrees.

				>>> t = lambda x: True
				>>> _matchtree(parse('A'), ('symbol', 'A'))
				[]
				>>> _matchtree(parse('A'), t)
				[('symbol', 'A')]
				>>> _matchtree(parse('A'), ('symbol', ('A',)))
				>>> _matchtree(parse('A'), ('symbol', 'A', 'B'))
				>>> _matchtree(parse('A'), ('func', 'A'))
				>>> _matchtree(parse('A'), ('symbol'))
				>>> _matchtree(parse('A'), 'A')
				>>> _matchtree(parse('A'), (t, lambda x: x == 'A'))
				['symbol', 'A']
				>>> _matchtree(parse('A'), (t, lambda x: x == 'B'))
				>>> _matchtree(parse('A+B'), (t, ('list', t, ('symbol', t))))
				['or', ('symbol', 'A'), 'B']
				"""
				matchedlist = []
				if util.safehasattr(pattern, '__call__'):
				matched = pattern(tree)
				if matched:
				return [tree]
				else:
				return None
				else:
				if isinstance(tree, tuple):
				if not isinstance(pattern, tuple) or len(tree) != len(pattern):
				return None
				for i, t in enumerate(tree):
				matched = _matchtree(t, pattern[i])
				if matched is None:
				return None
				matchedlist.extend(matched)
				elif tree != pattern:
				return None
				return matchedlist

	def _fixops(x):			def _fixops(x):
	"""Rewrite raw parsed tree to resolve ambiguous syntax which cannot be			"""Rewrite raw parsed tree to resolve ambiguous syntax which cannot be
	handled well by our simple top-down parser"""			handled well by our simple top-down parser"""
	if not isinstance(x, tuple):			if not isinstance(x, tuple):
	return x			return x

	op = x[0]			op = x[0]
	if op == 'parent':			if op == 'parent':
	op = x[0]			op = x[0]
	if op in ('string', 'symbol'):			if op in ('string', 'symbol'):
	return smallbonus, x # single revisions are small			return smallbonus, x # single revisions are small
	elif op == 'and':			elif op == 'and':
	wa, ta = _optimize(x[1], True)			wa, ta = _optimize(x[1], True)
	wb, tb = _optimize(x[2], True)			wb, tb = _optimize(x[2], True)
	w = min(wa, wb)			w = min(wa, wb)

				# (draft/secret/_notpublic() & ::x) has a fast path
				m = _match('_() & ancestors(_)', ('and', ta, tb))
				yujaUnsubmitted Not Done Perhaps `::x & phasename()` can be optimized, too. yuja: Perhaps `::x & phasename()` can be optimized, too.
				if m and getsymbol(m[1]) in {'draft', 'secret', '_notpublic'}:
				return w, _build('_phaseandancestors(_, _)', m[1], m[2])

	# (::x and not ::y)/(not ::y and ::x) have a fast path			# (::x and not ::y)/(not ::y and ::x) have a fast path
	m = _matchonly(ta, tb) or _matchonly(tb, ta)			m = _matchonly(ta, tb) or _matchonly(tb, ta)
	if m:			if m:
	return w, _build('only(_, _)', *m[1:])			return w, _build('only(_, _)', *m[1:])

	m = _match('not _', tb)			m = _match('not _', tb)
	if m:			if m:
	return wa, ('difference', ta, m[1])			return wa, ('difference', ta, m[1])

	if wa > wb:			if wa > wb:
	op = 'andsmally'			op = 'andsmally'
	return w, (op, ta, tb)			return w, (op, ta, tb)
	elif op == 'or':			elif op == 'or':
				yujaUnsubmitted Not Done FWIW, I wrote placeholder matcher once, which could be used as follows. m = _matchtree('_() & ::_', ('and', ta, tb)) if m and getsymbol(m[1]) in _fastphases: ... https://bitbucket.org/yuja/hg-work/commits/6c0ae642d86a188af8e940e430522e915f478b25 This is outdated and needs modification, but do you like it? yuja: FWIW, I wrote placeholder matcher once, which could be used as follows. ``` m = _matchtree('_…
				quarkAuthorUnsubmitted Not Done Yes! That's much shorter (I had to copy the trees from debugshell) and more robust (no fragile `len(x) >= 3` handling). For performance concerns, I think it's fine. Practically, revset parsing is hardly a bottleneck. With chg, `_treecache` could be pre-populated. quark: Yes! That's much shorter (I had to copy the trees from debugshell) and more robust (no fragile…
				yujaUnsubmitted Not Done I'll rebase my patches then. I'm sure that isn't trivial because I introduced the order flag. yuja: I'll rebase my patches then. I'm sure that isn't trivial because I introduced the order flag.
				quarkAuthorUnsubmitted Not Done Maybe `order` could be removed from AST since they can be inferred during tree traversal? I have a draft D451 that verifies the idea. quark: Maybe `order` could be removed from AST since they can be inferred during tree traversal? I…
	# fast path for machine-generated expression, that is likely to have			# fast path for machine-generated expression, that is likely to have
	# lots of trivial revisions: 'a + b + c()' to '_list(a b) + c()'			# lots of trivial revisions: 'a + b + c()' to '_list(a b) + c()'
	ws, ts, ss = [], [], []			ws, ts, ss = [], [], []
	def flushss():			def flushss():
	if not ss:			if not ss:
	return			return
	if len(ss) == 1:			if len(ss) == 1:
	w, t = ss[0]			w, t = ss[0]

tests/test-revset.t


	$ hg log -r 'successors(B+A)-contentdivergent()' -T '{desc}\n'			$ hg log -r 'successors(B+A)-contentdivergent()' -T '{desc}\n'
	A			A
	Z			Z
	B			B

	$ hg log -r 'successors(B+A)-contentdivergent()-obsolete()' -T '{desc}\n'			$ hg log -r 'successors(B+A)-contentdivergent()-obsolete()' -T '{desc}\n'
	Z			Z

				Test `draft() & ::x` optimization

				$ hg init $TESTTMP/repo2
				$ cd $TESTTMP/repo2
				$ hg debugdrawdag <<'EOS'
				> P5 S1
				> \| \|
				> S2 \| D3
				> \\|/
				> P4
				> \|
				> P3 D2
				> \| \|
				> P2 D1
				> \|/
				> P1
				> \|
				> P0
				> EOS
				$ hg phase --public -r P5
				quarkAuthorUnsubmitted Done Need to change to just `P5`. quark: Need to change to just `P5`.
				$ hg phase --force --secret -r S1+S2
				$ hg log -G -T '{rev} {desc} {phase}' -r 'sort(all(), topo, topo.firstbranch=P5)'
				o 8 P5 public
				\|
				\| o 10 S1 secret
				\| \|
				\| o 7 D3 draft
				\|/
				\| o 9 S2 secret
				\|/
				o 6 P4 public
				\|
				o 5 P3 public
				\|
				o 3 P2 public
				\|
				\| o 4 D2 draft
				\| \|
				\| o 2 D1 draft
				\|/
				o 1 P1 public
				\|
				o 0 P0 public

				$ hg debugrevspec --verify -p analyzed -p optimized 'draft() & ::(((S1+D1+P5)-D3)+S2)'
				* analyzed:
				(and
				(func
				('symbol', 'draft')
				None)
				(func
				('symbol', 'ancestors')
				(or
				(list
				(and
				(or
				(list
				('symbol', 'S1')
				('symbol', 'D1')
				('symbol', 'P5')))
				(not
				('symbol', 'D3')))
				('symbol', 'S2')))))
				* optimized:
				(func
				('symbol', '_phaseandancestors')
				(list
				('symbol', 'draft')
				(or
				(list
				(difference
				(func
				('symbol', '_list')
				('string', 'S1\x00D1\x00P5'))
				('symbol', 'D3'))
				('symbol', 'S2')))))
				$ hg debugrevspec --verify -p analyzed -p optimized 'secret() & ::9'
				* analyzed:
				(and
				(func
				('symbol', 'secret')
				None)
				(func
				('symbol', 'ancestors')
				('symbol', '9')))
				* optimized:
				(func
				('symbol', '_phaseandancestors')
				(list
				('symbol', 'secret')
				('symbol', '9')))
				$ hg debugrevspec --verify -p analyzed -p optimized '7 & ( (not public()) & ::(tag()) )'
				* analyzed:
				(and
				('symbol', '7')
				(and
				(not
				(func
				('symbol', 'public')
				None))
				(func
				('symbol', 'ancestors')
				(func
				('symbol', 'tag')
				None))))
				* optimized:
				(and
				('symbol', '7')
				(func
				('symbol', '_phaseandancestors')
				(list
				('symbol', '_notpublic')
				(func
				('symbol', 'tag')
				None))))
				$ hg debugrevspec --verify -p optimized '(not public()) & ancestors(S1+D2+P5, 1)'
				* optimized:
				(and
				(func
				('symbol', '_notpublic')
				None)
				(func
				('symbol', 'ancestors')
				(list
				(func
				('symbol', '_list')
				('string', 'S1\x00D2\x00P5'))
				('symbol', '1'))))
				$ hg debugrevspec --verify -p optimized '(not public()) & ancestors(S1+D2+P5, depth=1)'
				* optimized:
				(and
				(func
				('symbol', '_notpublic')
				None)
				(func
				('symbol', 'ancestors')
				(list
				(func
				('symbol', '_list')
				('string', 'S1\x00D2\x00P5'))
				(keyvalue
				('symbol', 'depth')
				('symbol', '1')))))

			Path	Packages
M			mercurial/dagop.py (30 lines)
M			mercurial/revset.py (30 lines)
M			mercurial/revsetlang.py (49 lines)
M			tests/test-revset.t (144 lines)

This is an archive of the discontinued Mercurial Phabricator instance.

revset: optimize "draft() & ::x" pattern
ClosedPublic

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 1570

mercurial/dagop.py

mercurial/revset.py

mercurial/revsetlang.py

tests/test-revset.t

This is an archive of the discontinued Mercurial Phabricator instance.

revset: optimize "draft() & ::x" patternClosedPublic

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 1570

mercurial/dagop.py

mercurial/revset.py

mercurial/revsetlang.py

tests/test-revset.t

revset: optimize "draft() & ::x" pattern
ClosedPublic

Revision Contents
Changeset List