This is an archive of the discontinued Mercurial Phabricator instance.

fsmonitor: warn when fsmonitor could be used
ClosedPublic

Authored by indygreg on Oct 1 2017, 6:49 PM.

Download Raw Diff

Details

Reviewers

mbthomas
krbullock
durin42

Group Reviewers

hg-reviewers

Commits

rHGdf2ff314e36f: fsmonitor: warn when fsmonitor could be used
rHGcb0a1764050c: fsmonitor: warn when fsmonitor could be used

Summary

fsmonitor can significantly speed up operations on large working
directories. But fsmonitor isn't enabled by default, so naive users
may not realize there is a potential to make Mercurial faster.

This commit introduces a warning to working directory updates when
fsmonitor could be used.

The following conditions must be met:

Working directory is previously empty
New working directory adds >= N files (currently 50,000)
Running on Linux or MacOS
fsmonitor not enabled
Warning not disabled via config override

Because of the empty working directory restriction, most users will
only see this warning during hg clone (assuming very few users
actually do an hg up null).

The addition of a warning may be considered a BC change. However, clone
has printed warnings before. Until recently, Mercurial printed a warning
with the server's certificate fingerprint when it wasn't explicitly
trusted for example. The warning goes to stderr. So it shouldn't
interfere with scripts parsing meaningful output.

The OS restriction was on the advice of Facebook engineers, who only
feel confident with watchman's stability on the supported platforms.

.. feature::

Print warning when fsmonitor isn't being used on a large repository

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.Oct 1 2017, 6:49 PM

Herald added a reviewer: hg-reviewers. · View Herald TranscriptOct 1 2017, 6:49 PM

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

LGTM

mercurial/merge.py
1969–1973	I'm not sure if we're planning to work towards PEP8, but it (and I) prefer using parentheses to break long if conditionals, rather than backslashes.

krbullock requested changes to this revision.Oct 4 2017, 10:52 AM

krbullock added a subscriber: krbullock.

krbullock added inline comments.

mercurial/merge.py
1969–1973	It's been our style basically forever to use parens instead of backslashes. I'm honestly shocked we don't have a check-code rule for this.
tests/test-fsmonitor-warning.t
9 ↗	(On Diff #2315)	We can't squeeze these into a test that's already set up? Brand-new test cases are discouraged, since so much time in our test suite is already burned on initializing repos.

This revision now requires changes to proceed.Oct 4 2017, 10:52 AM

quark added a subscriber: quark.Oct 17 2017, 7:40 PM

quark added inline comments.

tests/test-fsmonitor-warning.t
9 ↗	(On Diff #2315)	There was no fsmonitor related tests. So I think this is fine.

durin42 requested changes to this revision.Oct 18 2017, 4:11 PM

durin42 added a subscriber: durin42.

durin42 added inline comments.

mercurial/merge.py
1969–1973	Yes, definitely use parens and not \.
tests/test-fsmonitor-warning.t
9 ↗	(On Diff #2315)	You could easily fold this into any number of existing tests - really all you need is a repo with N files, and the ability to clone twice in a `#if` block.

indygreg updated this revision to Diff 2998.Oct 18 2017, 4:58 PM

durin42 accepted this revision.Oct 18 2017, 5:01 PM

Closed by commit rHGcb0a1764050c: fsmonitor: warn when fsmonitor could be used (authored by indygreg). · Explain WhyOct 18 2017, 5:05 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			hgext/fsmonitor/__init__.py (20 lines)
M			mercurial/configitems.py (6 lines)
M			mercurial/merge.py (33 lines)
M			tests/hghave.py (5 lines)
M			tests/test-clone.t (77 lines)

Diff	ID	Description	Created	Lint	Unit
Base		Base
Diff 1	2315		Oct 1 2017, 6:49 PM	★	★
Diff 2	2998		Oct 18 2017, 4:58 PM	★	★
Diff 3	2999	rHGcb0a1764050c8c343698c5141c957800868d50fd	Oct 18 2017, 4:57 PM	★	★

Status	Author	Revision
Closed	indygreg	D894 fsmonitor: warn when fsmonitor could be used
Closed	indygreg	D893 fsmonitor: use configitem
Closed	indygreg	D892 fsmonitor: access copymap in new location

Diff 2999

hgext/fsmonitor/init.py

	On a particular Linux system, for a real-world repository with over 400,000			On a particular Linux system, for a real-world repository with over 400,000
	files hosted on ext4, vanilla `hg status` takes 1.3 seconds. On the same			files hosted on ext4, vanilla `hg status` takes 1.3 seconds. On the same
	system, with fsmonitor it takes about 0.3 seconds.			system, with fsmonitor it takes about 0.3 seconds.

	fsmonitor requires no configuration -- it will tell Watchman about your			fsmonitor requires no configuration -- it will tell Watchman about your
	repository as necessary. You'll need to install Watchman from			repository as necessary. You'll need to install Watchman from
	https://facebook.github.io/watchman/ and make sure it is in your PATH.			https://facebook.github.io/watchman/ and make sure it is in your PATH.

				fsmonitor is incompatible with the largefiles and eol extensions, and
				will disable itself if any of those are active.

	The following configuration options exist:			The following configuration options exist:

	::			::

	[fsmonitor]			[fsmonitor]
	mode = {off, on, paranoid}			mode = {off, on, paranoid}

	When `mode = off`, fsmonitor will disable itself (similar to not loading the			When `mode = off`, fsmonitor will disable itself (similar to not loading the
	Whether or not to walk the whole repo ourselves when our cached state has been			Whether or not to walk the whole repo ourselves when our cached state has been
	invalidated, for example when Watchman has been restarted or .hgignore rules			invalidated, for example when Watchman has been restarted or .hgignore rules
	have been changed. Walking the repo in that case can result in competing for			have been changed. Walking the repo in that case can result in competing for
	I/O with Watchman. For large repos it is recommended to set this value to			I/O with Watchman. For large repos it is recommended to set this value to
	false. You may wish to set this to true if you have a very fast filesystem			false. You may wish to set this to true if you have a very fast filesystem
	that can outpace the IPC overhead of getting the result data for the full repo			that can outpace the IPC overhead of getting the result data for the full repo
	from Watchman. Defaults to false.			from Watchman. Defaults to false.

	fsmonitor is incompatible with the largefiles and eol extensions, and			::
	will disable itself if any of those are active.
				[fsmonitor]
				warn_when_unused = (boolean)

				Whether to print a warning during certain operations when fsmonitor would be
				beneficial to performance but isn't enabled.

				::

				[fsmonitor]
				warn_update_file_count = (integer)

				If ``warn_when_unused`` is set and fsmonitor isn't enabled, a warning will
				be printed during working directory updates if this many files will be
				created.
	'''			'''

	# Platforms Supported			# Platforms Supported
	# ===================			# ===================
	#			#
	# Linux: Stable. Watchman and fsmonitor are both known to work reliably,			# Linux: Stable. Watchman and fsmonitor are both known to work reliably,
	# even under severe loads.			# even under severe loads.
	#			#

mercurial/configitems.py

	default=True,			default=True,
	)			)
	coreconfigitem('format', 'usegeneraldelta',			coreconfigitem('format', 'usegeneraldelta',
	default=True,			default=True,
	)			)
	coreconfigitem('format', 'usestore',			coreconfigitem('format', 'usestore',
	default=True,			default=True,
	)			)
				coreconfigitem('fsmonitor', 'warn_when_unused',
				default=True,
				)
				coreconfigitem('fsmonitor', 'warn_update_file_count',
				default=50000,
				)
	coreconfigitem('hooks', '.*',			coreconfigitem('hooks', '.*',
	default=dynamicdefault,			default=dynamicdefault,
	generic=True,			generic=True,
	)			)
	coreconfigitem('hgweb-paths', '.*',			coreconfigitem('hgweb-paths', '.*',
	default=list,			default=list,
	generic=True,			generic=True,
	)			)

mercurial/merge.py

	modifiednodeid,			modifiednodeid,
	nullhex,			nullhex,
	nullid,			nullid,
	nullrev,			nullrev,
	)			)
	from . import (			from . import (
	copies,			copies,
	error,			error,
				extensions,
	filemerge,			filemerge,
	match as matchmod,			match as matchmod,
	obsutil,			obsutil,
	pycompat,			pycompat,
	scmutil,			scmutil,
	subrepo,			subrepo,
	util,			util,
	worker,			worker,
	### apply phase			### apply phase
	if not branchmerge: # just jump to the new rev			if not branchmerge: # just jump to the new rev
	fp1, fp2, xp1, xp2 = fp2, nullid, xp2, ''			fp1, fp2, xp1, xp2 = fp2, nullid, xp2, ''
	if not partial:			if not partial:
	repo.hook('preupdate', throw=True, parent1=xp1, parent2=xp2)			repo.hook('preupdate', throw=True, parent1=xp1, parent2=xp2)
	# note that we're in the middle of an update			# note that we're in the middle of an update
	repo.vfs.write('updatestate', p2.hex())			repo.vfs.write('updatestate', p2.hex())

				# Advertise fsmonitor when its presence could be useful.
				#
				# We only advertise when performing an update from an empty working
				# directory. This typically only occurs during initial clone.
				#
				# We give users a mechanism to disable the warning in case it is
				# annoying.
				#
				# We only allow on Linux and MacOS because that's where fsmonitor is
				# considered stable.
				fsmonitorwarning = repo.ui.configbool('fsmonitor', 'warn_when_unused')
				fsmonitorthreshold = repo.ui.configint('fsmonitor',
				'warn_update_file_count')
				try:
				extensions.find('fsmonitor')
				fsmonitorenabled = repo.ui.config('fsmonitor', 'mode') != 'off'
				# We intentionally don't look at whether fsmonitor has disabled
				# itself because a) fsmonitor may have already printed a warning
				# b) we only care about the config state here.
				except KeyError:
				fsmonitorenabled = False

				if (fsmonitorwarning
				and not fsmonitorenabled
				and p1.node() == nullid
				and len(actions['g']) >= fsmonitorthreshold
				and pycompat.sysplatform.startswith(('linux', 'darwin'))):
				mbthomasUnsubmitted Not Done I'm not sure if we're planning to work towards PEP8, but it (and I) prefer using parentheses to break long if conditionals, rather than backslashes. mbthomas: I'm not sure if we're planning to work towards PEP8, but it (and I) prefer using parentheses to…
				krbullockUnsubmitted Not Done It's been our style basically forever to use parens instead of backslashes. I'm honestly shocked we don't have a check-code rule for this. krbullock: It's been our style basically forever to use parens instead of backslashes. I'm honestly…
				durin42Unsubmitted Not Done Yes, definitely use parens and not \. durin42: Yes, definitely use parens and not \.
				repo.ui.warn(
				_('(warning: large working directory being used without '
				'fsmonitor enabled; enable fsmonitor to improve performance; '
				'see "hg help -e fsmonitor")\n'))

	stats = applyupdates(repo, actions, wc, p2, overwrite, labels=labels)			stats = applyupdates(repo, actions, wc, p2, overwrite, labels=labels)
	wc.flushall()			wc.flushall()

	if not partial:			if not partial:
	with repo.dirstate.parentchange():			with repo.dirstate.parentchange():
	repo.setparents(fp1, fp2)			repo.setparents(fp1, fp2)
	recordupdates(repo, actions, branchmerge)			recordupdates(repo, actions, branchmerge)
	# update completed, clear state			# update completed, clear state

tests/hghave.py

	'productbuild', br'Usage: productbuild ',			'productbuild', br'Usage: productbuild ',
	ignorestatus=1)			ignorestatus=1)
	and matchoutput('lsbom', br'Usage: lsbom', ignorestatus=1)			and matchoutput('lsbom', br'Usage: lsbom', ignorestatus=1)
	and matchoutput(			and matchoutput(
	'xar --help', br'Usage: xar', ignorestatus=1))			'xar --help', br'Usage: xar', ignorestatus=1))
	except ImportError:			except ImportError:
	return False			return False

				@check('linuxormacos', 'Linux or MacOS')
				def has_linuxormacos():
				# This isn't a perfect test for MacOS. But it is sufficient for our needs.
				return sys.platform.startswith(('linux', 'darwin'))

	@check("docker", "docker support")			@check("docker", "docker support")
	def has_docker():			def has_docker():
	pat = br'A self-sufficient runtime for'			pat = br'A self-sufficient runtime for'
	if matchoutput('docker --help', pat):			if matchoutput('docker --help', pat):
	if 'linux' not in sys.platform:			if 'linux' not in sys.platform:
	# TODO: in theory we should be able to test docker-based			# TODO: in theory we should be able to test docker-based
	# package creation on non-linux using boot2docker, but in			# package creation on non-linux using boot2docker, but in
	# practice that requires extra coordination to make sure			# practice that requires extra coordination to make sure

tests/test-clone.t

	sending hello command			sending hello command
	sending between command			sending between command
	abort: no suitable response from remote hg!			abort: no suitable response from remote hg!
	[255]			[255]

	We should not have created a file named owned - if it exists, the			We should not have created a file named owned - if it exists, the
	attack succeeded.			attack succeeded.
	$ if test -f owned; then echo 'you got owned'; fi			$ if test -f owned; then echo 'you got owned'; fi

				Cloning without fsmonitor enabled does not print a warning for small repos

				$ hg clone a fsmonitor-default
				updating to bookmark @ on branch stable
				3 files updated, 0 files merged, 0 files removed, 0 files unresolved

				Lower the warning threshold to simulate a large repo

				$ cat >> $HGRCPATH << EOF
				> [fsmonitor]
				> warn_update_file_count = 2
				> EOF

				We should see a warning about no fsmonitor on supported platforms

				#if linuxormacos no-fsmonitor
				$ hg clone a nofsmonitor
				updating to bookmark @ on branch stable
				(warning: large working directory being used without fsmonitor enabled; enable fsmonitor to improve performance; see "hg help -e fsmonitor")
				3 files updated, 0 files merged, 0 files removed, 0 files unresolved
				#else
				$ hg clone a nofsmonitor
				updating to bookmark @ on branch stable
				3 files updated, 0 files merged, 0 files removed, 0 files unresolved
				#endif

				We should not see warning about fsmonitor when it is enabled

				#if fsmonitor
				$ hg clone a fsmonitor-enabled
				updating to bookmark @ on branch stable
				3 files updated, 0 files merged, 0 files removed, 0 files unresolved
				#endif

				We can disable the fsmonitor warning

				$ hg --config fsmonitor.warn_when_unused=false clone a fsmonitor-disable-warning
				updating to bookmark @ on branch stable
				3 files updated, 0 files merged, 0 files removed, 0 files unresolved

				Loaded fsmonitor but disabled in config should still print warning

				#if linuxormacos fsmonitor
				$ hg --config fsmonitor.mode=off clone a fsmonitor-mode-off
				updating to bookmark @ on branch stable
				(warning: large working directory being used without fsmonitor enabled; enable fsmonitor to improve performance; see "hg help -e fsmonitor") (fsmonitor !)
				3 files updated, 0 files merged, 0 files removed, 0 files unresolved
				#endif

				Warning not printed if working directory isn't empty

				$ hg -q clone a fsmonitor-update
				(warning: large working directory being used without fsmonitor enabled; enable fsmonitor to improve performance; see "hg help -e fsmonitor") (?)
				$ cd fsmonitor-update
				$ hg up acb14030fe0a
				1 files updated, 0 files merged, 2 files removed, 0 files unresolved
				(leaving bookmark @)
				$ hg up cf0fe1914066
				1 files updated, 0 files merged, 0 files removed, 0 files unresolved

				`hg update` from null revision also prints

				$ hg up null
				0 files updated, 0 files merged, 2 files removed, 0 files unresolved

				#if linuxormacos no-fsmonitor
				$ hg up cf0fe1914066
				(warning: large working directory being used without fsmonitor enabled; enable fsmonitor to improve performance; see "hg help -e fsmonitor")
				2 files updated, 0 files merged, 0 files removed, 0 files unresolved
				#else
				$ hg up cf0fe1914066
				2 files updated, 0 files merged, 0 files removed, 0 files unresolved
				#endif

				$ cd ..