This is an archive of the discontinued Mercurial Phabricator instance.

Differential D805

alias: make alias command lazily resolved
ClosedPublic

Authored by quark on Sep 23 2017, 4:57 PM.

Download Raw Diff

Details

Reviewers

yuja

Group Reviewers

hg-reviewers

Commits

rHGbd50aa1aa035: alias: make alias command lazily resolved

Summary

With many aliases, resolving them could have some visible overhead. Below is
part of traceprof [1] output of hg bookmark --hidden:

(time unit: ms)
37  \ addaliases                             dispatch.py:526
37   | __init__ (60 times)                   dispatch.py:402
33   | findcmd (108 times)                   cmdutil.py:721
16   | findpossible (49 times)               cmdutil.py:683

It may get better by optimizing findcmd to do a bisect, but we don't
really need to resolve an alias if it's not used, so let's make those
command entries lazy.

After this patch, addalias takes less than 1ms.

.. perf:: improved performance when many aliases are defined

[1]: https://bitbucket.org/facebook/hg-experimental/src/9aca0dbdbdfc48457e5d2581ca2d6e662fced2e6/hgext3rd/traceprof.pyx

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

quark created this revision.Sep 23 2017, 4:57 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptSep 23 2017, 4:57 PM

Herald added a subscriber: mercurial-devel. · View Herald Transcript

quark edited the summary of this revision. (Show Details)Sep 23 2017, 5:00 PM

quark updated this revision to Diff 2050.Sep 23 2017, 5:34 PM

quark updated this revision to Diff 2052.Sep 23 2017, 5:41 PM

Looks okay. Queued, thanks.

mercurial/dispatch.py
539	A slice operation would be unlikely for alias commands, so this should be fine.

This revision is now accepted and ready to land.Sep 25 2017, 10:48 AM

Closed by commit rHGbd50aa1aa035: alias: make alias command lazily resolved (authored by quark). · Explain WhySep 25 2017, 11:02 AM

This revision was automatically updated to reflect the committed changes.

quark mentioned this in D912: test-alias: make it compatible with chg.Oct 2 2017, 7:59 PM

quark mentioned this in rHGa8a9c0d3232b: test-alias: make it compatible with chg.Oct 5 2017, 10:01 AM

Revision Contents
Changeset List

			Path	Packages
M			mercurial/dispatch.py (37 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	2049		Sep 23 2017, 4:57 PM	★	★
Diff 2	2050		Sep 23 2017, 5:34 PM	★	★
Diff 3	2052		Sep 23 2017, 5:41 PM	★	★
Diff 4	2065	rHGbd50aa1aa035429e7cd099aae8f3b2db388e561b	Sep 23 2017, 4:46 PM	★	★

	Status	Author	Revision
	Closed	quark	D805 alias: make alias command lazily resolved
	Closed	quark	D804 alias: test duplicated definition earlier

Diff 2065

mercurial/dispatch.py

	else:			else:
	try:			try:
	return util.checksignature(self.fn)(ui, args, *opts)			return util.checksignature(self.fn)(ui, args, *opts)
	except error.SignatureError:			except error.SignatureError:
	args = ' '.join([self.cmdname] + self.args)			args = ' '.join([self.cmdname] + self.args)
	ui.debug("alias '%s' expands to '%s'\n" % (self.name, args))			ui.debug("alias '%s' expands to '%s'\n" % (self.name, args))
	raise			raise

				class lazyaliasentry(object):
				"""like a typical command entry (func, opts, help), but is lazy"""

				def __init__(self, name, definition, cmdtable, source):
				self.name = name
				self.definition = definition
				self.cmdtable = cmdtable.copy()
				self.source = source

				@util.propertycache
				def _aliasdef(self):
				return cmdalias(self.name, self.definition, self.cmdtable, self.source)

				def __getitem__(self, n):
				yujaUnsubmitted Not Done A slice operation would be unlikely for alias commands, so this should be fine. yuja: A slice operation would be unlikely for alias commands, so this should be fine.
				aliasdef = self._aliasdef
				if n == 0:
				return aliasdef
				elif n == 1:
				return aliasdef.opts
				elif n == 2:
				return aliasdef.help
				else:
				raise IndexError

				def __iter__(self):
				for i in range(3):
				yield self[i]

				def __len__(self):
				return 3

	def addaliases(ui, cmdtable):			def addaliases(ui, cmdtable):
	# aliases are processed after extensions have been loaded, so they			# aliases are processed after extensions have been loaded, so they
	# may use extension commands. Aliases can also use other alias definitions,			# may use extension commands. Aliases can also use other alias definitions,
	# but only if they have been defined prior to the current definition.			# but only if they have been defined prior to the current definition.
	for alias, definition in ui.configitems('alias'):			for alias, definition in ui.configitems('alias'):
	try:			try:
	if cmdtable[alias][0].definition == definition:			if cmdtable[alias].definition == definition:
	continue			continue
	except (KeyError, AttributeError):			except (KeyError, AttributeError):
	# definition might not exist or it might not be a cmdalias			# definition might not exist or it might not be a cmdalias
	pass			pass

	source = ui.configsource('alias', alias)			source = ui.configsource('alias', alias)
	aliasdef = cmdalias(alias, definition, cmdtable, source)			entry = lazyaliasentry(alias, definition, cmdtable, source)
	cmdtable[aliasdef.name] = (aliasdef, aliasdef.opts, aliasdef.help)			cmdtable[alias] = entry

	def _parse(ui, args):			def _parse(ui, args):
	options = {}			options = {}
	cmdoptions = {}			cmdoptions = {}

	try:			try:
	args = fancyopts.fancyopts(args, commands.globalopts, options)			args = fancyopts.fancyopts(args, commands.globalopts, options)
	except getopt.GetoptError as inst:			except getopt.GetoptError as inst: