D894: fsmonitor: warn when fsmonitor could be used
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.
REVISION 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
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D894
AFFECTED FILES
hgext/fsmonitor/__init__.py
mercurial/configitems.py
mercurial/merge.py
tests/hghave.py
tests/test-fsmonitor-warning.t
CHANGE DETAILS
diff --git a/tests/test-fsmonitor-warning.t b/tests/test-fsmonitor-warning.t
new file mode 100644
--- /dev/null
+++ b/tests/test-fsmonitor-warning.t
@@ -0,0 +1,94 @@
+Cloning without fsmonitor enabled does not print a warning for small repos
+
+ $ hg init source
+ $ cd source
+ $ touch file1 file2 file3 file4
+ $ hg -q commit -A -m initial
+ $ touch file5
+ $ hg -q commit -A -m 'add file5'
+ $ cd ..
+
+ $ hg clone source dest-default
+ updating to branch default
+ 5 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 = 4
+ > EOF
+
+We should see a warning about no fsmonitor on supported platforms
+
+#if linuxormacos no-fsmonitor
+
+ $ hg clone source dest-nofsmonitor
+ updating to branch default
+ (warning: large working directory being used without fsmonitor enabled;
enable fsmonitor to improve performance; see "hg help -e fsmonitor")
+ 5 files updated, 0 files merged, 0 files removed, 0 files unresolved
+
+#else
+
+ $ hg clone source dest-nofsmonitor
+ updating to branch default
+ 5 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 source dest-fsmonitor
+ updating to branch default
+ 5 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 source
dest-disable-warning
+ updating to branch default
+ 5 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 source dest-mode-off
+ updating to branch default
+ (warning: large working directory being used without fsmonitor enabled;
enable fsmonitor to improve performance; see "hg help -e fsmonitor") (fsmonitor
!)
+ 5 files updated, 0 files merged, 0 files removed, 0 files unresolved
+
+#endif
+
+Warning not printed if working directory isn't empty
+
+ $ hg -q clone source dest-update
+ (warning: large working directory being used without fsmonitor enabled;
enable fsmonitor to improve performance; see "hg help -e fsmonitor") (?)
+ $ cd dest-update
+ $ hg up 4c436bafb4ab
+ 0 files updated, 0 files merged, 1 files removed, 0 files unresolved
+ $ hg up 9637229078ea
+ 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, 5 files removed, 0 files unresolved
+
+#if linuxormacos no-fsmonitor
+
+ $ hg up 9637229078ea
+ (warning: large working directory being used without fsmonitor enabled;
enable fsmonitor to improve performance; see "hg help -e fsmonitor")
+ 5 files updated, 0 files merged, 0 files removed, 0 files unresolved
+
+#else
+
+ $ hg up 9637229078ea
+ 5 files updated, 0 files merged, 0 files removed, 0 files unresolved
+
+#endif
+
+ $ cd ..
diff --git a/tests/hghave.py b/tests/hghave.py
--- a/tests/hghave.py
+++ b/tests/hghave.py
@@ -554,6 +554,11 @@
except ImportError:
return False
+@check('linuxormacos
D894: fsmonitor: warn when fsmonitor could be used
mbthomas accepted this revision.
mbthomas added a comment.
LGTM
INLINE COMMENTS
> merge.py:1739-1743
> +if fsmonitorwarning \
> +and not fsmonitorenabled \
> +and p1.node() == nullid \
> +and len(actions['g']) >= fsmonitorthreshold \
> +and pycompat.sysplatform.startswith(('linux', 'darwin')):
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.
REPOSITORY
rHG Mercurial
REVISION DETAIL
https://phab.mercurial-scm.org/D894
To: indygreg, #hg-reviewers, mbthomas
Cc: mbthomas, mercurial-devel
___
Mercurial-devel mailing list
Mercurial-devel@mercurial-scm.org
https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D894: fsmonitor: warn when fsmonitor could be used
krbullock requested changes to this revision. krbullock added inline comments. This revision now requires changes to proceed. INLINE COMMENTS > mbthomas wrote in merge.py:1739-1743 > 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. 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. > test-fsmonitor-warning.t:9 > + $ hg -q commit -A -m 'add file5' > + $ cd .. > + 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. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D894 To: indygreg, #hg-reviewers, mbthomas, krbullock Cc: krbullock, mbthomas, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D894: fsmonitor: warn when fsmonitor could be used
quark added inline comments. INLINE COMMENTS > krbullock wrote in test-fsmonitor-warning.t:9 > 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. There was no fsmonitor related tests. So I think this is fine. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D894 To: indygreg, #hg-reviewers, mbthomas, krbullock Cc: quark, krbullock, mbthomas, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D894: fsmonitor: warn when fsmonitor could be used
durin42 requested changes to this revision. durin42 added inline comments. INLINE COMMENTS > krbullock wrote in merge.py:1739-1743 > 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. Yes, definitely use parens and not \. > quark wrote in test-fsmonitor-warning.t:9 > There was no fsmonitor related tests. So I think this is fine. 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. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D894 To: indygreg, #hg-reviewers, mbthomas, krbullock, durin42 Cc: durin42, quark, krbullock, mbthomas, mercurial-devel ___ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
D894: fsmonitor: warn when fsmonitor could be used
indygreg updated this revision to Diff 2998.
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D894?vs=2315&id=2998
REVISION DETAIL
https://phab.mercurial-scm.org/D894
AFFECTED FILES
hgext/fsmonitor/__init__.py
mercurial/configitems.py
mercurial/merge.py
tests/hghave.py
tests/test-clone.t
CHANGE DETAILS
diff --git a/tests/test-clone.t b/tests/test-clone.t
--- a/tests/test-clone.t
+++ b/tests/test-clone.t
@@ -1177,3 +1177,80 @@
We should not have created a file named owned - if it exists, the
attack succeeded.
$ 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 ..
+
diff --git a/tests/hghave.py b/tests/hghave.py
--- a/tests/hghave.py
+++ b/tests/hghave.py
@@ -559,6 +559,11 @@
except ImportError:
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")
def has_docker():
pat = br'A self-sufficient runtime for'
diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -25,6 +25,7 @@
from . import (
copies,
error,
+extensions,
filemerge,
match as matchmod,
obsutil,
@@ -1943,6 +1944,38 @@
# note that we're in the middle of an update
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')
+
D894: fsmonitor: warn when fsmonitor could be used
This revision was automatically updated to reflect the committed changes.
Closed by commit rHGcb0a1764050c: fsmonitor: warn when fsmonitor could be used
(authored by indygreg, committed by ).
REPOSITORY
rHG Mercurial
CHANGES SINCE LAST UPDATE
https://phab.mercurial-scm.org/D894?vs=2998&id=2999
REVISION DETAIL
https://phab.mercurial-scm.org/D894
AFFECTED FILES
hgext/fsmonitor/__init__.py
mercurial/configitems.py
mercurial/merge.py
tests/hghave.py
tests/test-clone.t
CHANGE DETAILS
diff --git a/tests/test-clone.t b/tests/test-clone.t
--- a/tests/test-clone.t
+++ b/tests/test-clone.t
@@ -1177,3 +1177,80 @@
We should not have created a file named owned - if it exists, the
attack succeeded.
$ 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 ..
+
diff --git a/tests/hghave.py b/tests/hghave.py
--- a/tests/hghave.py
+++ b/tests/hghave.py
@@ -559,6 +559,11 @@
except ImportError:
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")
def has_docker():
pat = br'A self-sufficient runtime for'
diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -25,6 +25,7 @@
from . import (
copies,
error,
+extensions,
filemerge,
match as matchmod,
obsutil,
@@ -1943,6 +1944,38 @@
# note that we're in the middle of an update
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_u
