mercurial/revset.py
1554	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
1554	Good point. Thanks!

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

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

mercurial/revsetlang.py
492	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
492	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
492	I'll rebase my patches then. I'm sure that isn't trivial because I introduced the order flag.

mercurial/revsetlang.py
492	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
1558	`revs & phasecache.getrevset(repo, [phasename])` might be slightly faster, but I don't think it would make any noticeable difference.
mercurial/revsetlang.py
475	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 1074

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, keepfunc):
	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 keepfunc is None:
				pfunc = plainpfunc
				else:
				pfunc = lambda rev: filter(keepfunc, plainpfunc(rev))
				revs = revs.filter(keepfunc)
				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, keepfunc=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 keepfunc is provided, it will be used to cut the traversal of the DAG.
				When keepfunc(X) returns False, the DAG traversal stops - revision X and
				X's ancestors in the traversal path will be skipped.
	"""			"""
	gen = _genrevancestors(repo, revs, followfirst, startdepth, stopdepth)			gen = _genrevancestors(repo, revs, followfirst, startdepth, stopdepth,
				keepfunc)
	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 = getstring(args[0], _("phasename needs to be a string"))
				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 keepfunc(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, keepfunc=keepfunc)

				if phasename == 'draft': # need to remove secret changesets
				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…
				revs = revs.filter(lambda r: getphase(repo, r) == draft)
				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

	>>> f('ancestors(A)', 'not ancestors(B)')			>>> f('ancestors(A)', 'not ancestors(B)')
	('list', ('symbol', 'A'), ('symbol', 'B'))			('list', ('symbol', 'A'), ('symbol', 'B'))
	"""			"""
	ta = _matchnamedfunc(revs, 'ancestors')			ta = _matchnamedfunc(revs, 'ancestors')
	tb = bases and bases[0] == 'not' and _matchnamedfunc(bases[1], 'ancestors')			tb = bases and bases[0] == 'not' and _matchnamedfunc(bases[1], 'ancestors')
	if _isposargs(ta, 1) and _isposargs(tb, 1):			if _isposargs(ta, 1) and _isposargs(tb, 1):
	return ('list', ta, tb)			return ('list', ta, tb)

				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':
	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)
	order = x[3]			order = x[3]
	w = min(wa, wb)			w = min(wa, wb)

	# (::x and not ::y)/(not ::y and ::x) have a fast path			# (::x and not ::y)/(not ::y and ::x) have a fast path
	tm = _matchonly(ta, tb) or _matchonly(tb, ta)			tm = _matchonly(ta, tb) or _matchonly(tb, ta)
				yujaUnsubmitted Not Done Perhaps `::x & phasename()` can be optimized, too. yuja: Perhaps `::x & phasename()` can be optimized, too.
	if tm:			if tm:
	return w, ('func', ('symbol', 'only'), tm, order)			return w, ('func', ('symbol', 'only'), tm, order)

	if tb is not None and tb[0] == 'not':			if tb is not None and tb[0] == 'not':
	return wa, ('difference', ta, tb[1], order)			return wa, ('difference', ta, tb[1], order)

				# (draft/secret/_notpublic() & ::x) has a fast path
				_fastphases = {'draft', 'secret', '_notpublic'}
				matched = _matchtree(
				(ta, tb),
				(('func', ('symbol', _fastphases.__contains__), None,
				('define', 'any').__contains__),
				('func', ('symbol', 'ancestors'),
				# do not match if "depth" is set for "ancestors"
				lambda x: not (x[0] == 'list' and len(x) >= 3),
				'follow')))
				if matched:
				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…
				return w, ('func', ('symbol', '_phaseandancestors'),
				('list', ('string', matched[0]), matched[2]), 'define')

	if wa > wb:			if wa > wb:
	return w, (op, tb, ta, order)			return w, (op, tb, ta, order)
	return w, (op, ta, tb, order)			return w, (op, ta, tb, order)
	elif op == 'or':			elif op == 'or':
	# 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()'
	order = x[2]			order = x[2]
	ws, ts, ss = [], [], []			ws, ts, ss = [], [], []

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

			Path	Packages
M			mercurial/dagop.py (19 lines)
M			mercurial/revset.py (31 lines)
M			mercurial/revsetlang.py (57 lines)
M			tests/test-revset.t (176 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 1074

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 1074

mercurial/dagop.py

mercurial/revset.py

mercurial/revsetlang.py

tests/test-revset.t

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

Revision Contents
Changeset List