Merge pull request #21762 from ccordoba12/workspace-watcher-performance

PR: Improve performance of workspace watcher (Projects)

spyder/config/utils.py

@@ -151,11 +151,10 @@ def get_edit_filters():
 
 def get_edit_extensions():
     """
-    Return extensions associated with the file types
-    supported by the Editor
+    Return extensions associated with the file types supported by the Editor.
     """
     edit_filetypes = get_edit_filetypes(ignore_pygments_extensions=False)
-    return _get_extensions(edit_filetypes) + ['']
+    return _get_extensions(edit_filetypes)
 
 
 #==============================================================================

spyder/plugins/projects/tests/test_plugin.py

@@ -24,7 +24,6 @@
 
 # Local imports
 from spyder.app.cli_options import get_options
-from spyder.config.base import running_in_ci
 from spyder.config.manager import CONF
 import spyder.plugins.base
 from spyder.plugins.preferences.tests.conftest import MainWindowMock
@@ -368,27 +367,26 @@ def test_project_explorer_tree_root(projects, tmpdir, qtbot):
 
 
 @flaky(max_runs=5)
-@pytest.mark.skipif(sys.platform == 'darwin', reason="Fails on Mac")
-@pytest.mark.skipif(not running_in_ci(), reason="Hangs locally sometimes")
 def test_filesystem_notifications(qtbot, projects, tmpdir):
     """
     Test that filesystem notifications are emitted when creating,
     deleting and moving files and directories.
     """
     # Create a directory for the project and some files.
     project_root = tmpdir.mkdir('project0')
+    git_folder = project_root.mkdir('.git')
     folder0 = project_root.mkdir('folder0')
     folder1 = project_root.mkdir('folder1')
-    file0 = project_root.join('file0')
-    file1 = folder0.join('file1')
-    file2 = folder0.join('file2')
-    file3 = folder1.join('file3')
+    file0 = project_root.join('file0.txt')
+    file1 = folder0.join('file1.txt')
+    file2 = folder0.join('file2.txt')
+    file3 = folder1.join('file3.txt')
     file0.write('')
     file1.write('')
     file3.write('ab')
 
     # Open the project
-    projects.open_project(path=to_text_string(project_root))
+    projects.open_project(path=str(project_root))
 
     # Get a reference to the filesystem event handler
     fs_handler = projects.get_widget().watcher.event_handler
@@ -399,7 +397,7 @@ def test_filesystem_notifications(qtbot, projects, tmpdir):
         file2.write('')
 
     file_created, is_dir = blocker.args
-    assert file_created == to_text_string(file2)
+    assert file_created == str(file2)
     assert not is_dir
 
     # Test folder creation
@@ -408,57 +406,71 @@ def test_filesystem_notifications(qtbot, projects, tmpdir):
         folder2 = project_root.mkdir('folder2')
 
     folder_created, is_dir = blocker.args
-    assert folder_created == osp.join(to_text_string(project_root), 'folder2')
+    assert folder_created == osp.join(str(project_root), 'folder2')
 
     # Test file move/renaming
-    new_file = osp.join(to_text_string(folder0), 'new_file')
+    new_file = osp.join(str(folder0), 'new_file.txt')
     with qtbot.waitSignal(fs_handler.sig_file_moved,
                           timeout=3000) as blocker:
-        shutil.move(to_text_string(file1), new_file)
+        shutil.move(str(file1), new_file)
 
     original_file, file_moved, is_dir = blocker.args
-    assert original_file == to_text_string(file1)
+    assert original_file == str(file1)
     assert file_moved == new_file
     assert not is_dir
 
     # Test folder move/renaming
-    new_folder = osp.join(to_text_string(project_root), 'new_folder')
+    new_folder = osp.join(str(project_root), 'new_folder')
     with qtbot.waitSignal(fs_handler.sig_file_moved,
                           timeout=3000) as blocker:
-        shutil.move(to_text_string(folder2), new_folder)
+        shutil.move(str(folder2), new_folder)
 
     original_folder, folder_moved, is_dir = blocker.args
-    assert original_folder == to_text_string(folder2)
+    assert original_folder == str(folder2)
     assert folder_moved == new_folder
     assert is_dir
 
     # Test file deletion
     with qtbot.waitSignal(fs_handler.sig_file_deleted,
                           timeout=3000) as blocker:
-        os.remove(to_text_string(file0))
+        os.remove(str(file0))
 
     deleted_file, is_dir = blocker.args
-    assert deleted_file == to_text_string(file0)
+    assert deleted_file == str(file0)
     assert not is_dir
-    assert not osp.exists(to_text_string(file0))
+    assert not osp.exists(str(file0))
 
     # Test folder deletion
     with qtbot.waitSignal(fs_handler.sig_file_deleted,
                           timeout=3000) as blocker:
-        shutil.rmtree(to_text_string(folder0))
+        shutil.rmtree(str(folder0))
 
     deleted_folder, is_dir = blocker.args
-    assert to_text_string(folder0) in deleted_folder
+    assert str(folder0) in deleted_folder
 
-    # For some reason this fails in macOS
-    if not sys.platform == 'darwin':
-        # Test file/folder modification
-        with qtbot.waitSignal(fs_handler.sig_file_modified,
-                              timeout=3000) as blocker:
-            file3.write('abc')
-
-        modified_file, is_dir = blocker.args
-        assert modified_file in to_text_string(file3)
+    # Test file/folder modification
+    with qtbot.waitSignal(fs_handler.sig_file_modified,
+                          timeout=3000) as blocker:
+        file3.write('abc')
+
+    modified_file, is_dir = blocker.args
+    assert modified_file in str(file3)
+
+    # Test events in hidden folders are not emitted
+    with qtbot.assertNotEmitted(fs_handler.sig_file_created, wait=2000):
+        git_file = git_folder.join('git_file.txt')
+        git_file.write("Some data")
+
+    # Test events in pycache folders are not emitted
+    with qtbot.assertNotEmitted(fs_handler.sig_file_created, wait=2000):
+        pycache_folder = project_root.mkdir('__pycache__')
+        pycache_file = pycache_folder.join("foo.pyc")
+        pycache_file.write("")
+
+    # Test events for files with binary extensions are not emitted
+    with qtbot.assertNotEmitted(fs_handler.sig_file_created, wait=2000):
+        png_file = project_root.join("binary.png")
+        png_file.write("")
 
 
 def test_loaded_and_closed_signals(create_projects, tmpdir, mocker, qtbot):

spyder/plugins/projects/utils/watcher.py

@@ -7,23 +7,35 @@
 """Watcher to detect filesystem changes in the project's directory."""
 
 # Standard lib imports
+import os
 import logging
+from pathlib import Path
 
 # Third-party imports
 from qtpy.QtCore import QObject, Signal
-from qtpy.QtWidgets import QMessageBox
-
+from superqt.utils import qthrottled
 import watchdog
-from watchdog.observers import Observer
-from watchdog.events import FileSystemEventHandler
+from watchdog.events import FileSystemEventHandler, PatternMatchingEventHandler
+from watchdog.observers.polling import PollingObserverVFS
 
 # Local imports
-from spyder.config.base import _
-from spyder.py3compat import to_text_string
+from spyder.config.utils import get_edit_extensions
+
 
+# ---- Constants
+# -----------------------------------------------------------------------------
 logger = logging.getLogger(__name__)
 
+EDIT_EXTENSIONS = get_edit_extensions()
+
+FOLDERS_TO_IGNORE = [
+    "__pycache__",
+    "build",
+]
+
 
+# ---- Monkey patches
+# -----------------------------------------------------------------------------
 class BaseThreadWrapper(watchdog.utils.BaseThread):
     """
     Wrapper around watchdog BaseThread class.
@@ -50,7 +62,45 @@ def run_wrapper(self):
 watchdog.utils.BaseThread = BaseThreadWrapper
 
 
-class WorkspaceEventHandler(QObject, FileSystemEventHandler):
+# ---- Auxiliary functions
+# -----------------------------------------------------------------------------
+def ignore_entry(entry: os.DirEntry) -> bool:
+    """Check if an entry should be ignored."""
+    parts = Path(entry.path).parts
+
+    # Ignore files in hidden directories (e.g. .git)
+    if any([p.startswith(".") for p in parts]):
+        return True
+
+    # Ignore specific folders
+    for folder in FOLDERS_TO_IGNORE:
+        if folder in parts:
+            return True
+
+    return False
+
+
+def editable_file(entry: os.DirEntry) -> bool:
+    """Check if an entry file is editable."""
+    if entry.is_file():
+        return (os.path.splitext(entry.path)[1] in EDIT_EXTENSIONS)
+    return True
+
+
+def filter_scandir(path):
+    """
+    Filter entries from os.scandir that we're not interested in tracking in the
+    observer.
+    """
+    return (
+        entry for entry in os.scandir(path)
+        if (not ignore_entry(entry) and editable_file(entry))
+    )
+
+
+# ---- Event handler
+# -----------------------------------------------------------------------------
+class WorkspaceEventHandler(QObject, PatternMatchingEventHandler):
     """
     Event handler for watchdog notifications.
 
@@ -65,7 +115,10 @@ class WorkspaceEventHandler(QObject, FileSystemEventHandler):
 
     def __init__(self, parent=None):
         QObject.__init__(self, parent)
-        FileSystemEventHandler.__init__(self)
+        PatternMatchingEventHandler.__init__(
+            self,
+            patterns=[f"*{ext}" for ext in EDIT_EXTENSIONS],
+        )
 
     def fmt_is_dir(self, is_dir):
         return 'directory' if is_dir else 'file'
@@ -99,7 +152,16 @@ def on_modified(self, event):
             self.fmt_is_dir(is_dir), src_path))
         self.sig_file_modified.emit(src_path, is_dir)
 
+    def dispatch(self, event):
+        # Don't apply patterns to directories, only to files
+        if event.is_directory:
+            FileSystemEventHandler.dispatch(self, event)
+        else:
+            super().dispatch(event)
 
+
+# ---- Watcher
+# -----------------------------------------------------------------------------
 class WorkspaceWatcher(QObject):
     """
     Wrapper class around watchdog observer and notifier.
@@ -109,53 +171,49 @@ class WorkspaceWatcher(QObject):
 
     observer = None
 
+    sig_file_moved = Signal(str, str, bool)
+    sig_file_created = Signal(str, bool)
+    sig_file_deleted = Signal(str, bool)
+    sig_file_modified = Signal(str, bool)
+
     def __init__(self, parent=None):
         super().__init__(parent)
         self.event_handler = WorkspaceEventHandler(self)
 
+        self.event_handler.sig_file_moved.connect(self.on_moved)
+        self.event_handler.sig_file_created.connect(self.on_created)
+        self.event_handler.sig_file_deleted.connect(self.on_deleted)
+        self.event_handler.sig_file_modified.connect(self.on_modified)
+
     def connect_signals(self, project):
-        self.event_handler.sig_file_created.connect(project.file_created)
-        self.event_handler.sig_file_moved.connect(project.file_moved)
-        self.event_handler.sig_file_deleted.connect(project.file_deleted)
-        self.event_handler.sig_file_modified.connect(project.file_modified)
+        self.sig_file_created.connect(project.file_created)
+        self.sig_file_moved.connect(project.file_moved)
+        self.sig_file_deleted.connect(project.file_deleted)
+        self.sig_file_modified.connect(project.file_modified)
 
     def start(self, workspace_folder):
-        # Needed to handle an error caused by the inotify limit reached.
-        # See spyder-ide/spyder#10478
+        # We use a polling observer because:
+        # * It doesn't introduce long freezes on Linux when switching git
+        #   branches that have many changes between them. That's because the
+        #   OS-based observer (i.e. inotify) generates way too many events.
+        # * The OS-based observer on Windows has many shortcomings (see
+        #   openmsi/openmsistream#56).
+        # * There doesn't seem to be issues on Mac, but it's simpler to use a
+        #   single observer for all OSes.
+        self.observer = PollingObserverVFS(
+            stat=os.stat, listdir=filter_scandir
+        )
+
+        self.observer.schedule(
+            self.event_handler, workspace_folder, recursive=True
+        )
+
         try:
-            self.observer = Observer()
-            self.observer.schedule(
-                self.event_handler, workspace_folder, recursive=True)
-            try:
-                self.observer.start()
-            except OSError:
-                # This error happens frequently on Linux
-                logger.debug("Watcher could not be started.")
-        except OSError as e:
-            self.observer = None
-            if u'inotify' in to_text_string(e):
-                QMessageBox.warning(
-                    self.parent(),
-                    "Spyder",
-                    _("File system changes for this project can't be tracked "
-                      "because it contains too many files. To fix this you "
-                      "need to increase the inotify limit in your system, "
-                      "with the following command:"
-                      "<br><br>"
-                      "<code>"
-                      "sudo sysctl -n -w fs.inotify.max_user_watches=524288"
-                      "</code>"
-                      "<br><br>For a permanent solution you need to add to"
-                      "<code>/etc/sysctl.conf</code>"
-                      "the following line:<br><br>"
-                      "<code>"
-                      "fs.inotify.max_user_watches=524288"
-                      "</code>"
-                      "<br><br>"
-                      "After doing that, you need to close and start Spyder "
-                      "again so those changes can take effect."))
-            else:
-                raise e
+            self.observer.start()
+        except Exception:
+            logger.debug(
+                f"Observer could not be started for: {workspace_folder}."
+            )
 
     def stop(self):
         if self.observer is not None:
@@ -169,3 +227,19 @@ def stop(self):
                 self.observer = None
             except RuntimeError:
                 pass
+
+    @qthrottled(timeout=200)
+    def on_moved(self, src_path, dest_path, is_dir):
+        self.sig_file_moved.emit(src_path, dest_path, is_dir)
+
+    @qthrottled(timeout=200)
+    def on_created(self, path, is_dir):
+        self.sig_file_created.emit(path, is_dir)
+
+    @qthrottled(timeout=200)
+    def on_deleted(self, path, is_dir):
+        self.sig_file_deleted.emit(path, is_dir)
+
+    @qthrottled(timeout=200)
+    def on_modified(self, path, is_dir):
+        self.sig_file_modified.emit(path, is_dir)