Merge pull request #1317 from tangkong/perf_preset_loading

PERF: delay preset sync until tab completion / attribute access, track file desync

docs/source/upcoming_release_notes/1317-perf_preset_loading.rst

@@ -0,0 +1,31 @@
+1317 perf_preset_loading
+########################
+
+API Breaks
+----------
+- N/A
+
+Library Features
+----------------
+- N/A
+
+Device Features
+---------------
+- N/A
+
+New Devices
+-----------
+- N/A
+
+Bugfixes
+--------
+- N/A
+
+Maintenance
+-----------
+- Adds option to defer preset path loading until needed.  Presets will
+  now load when tab-completion or preset-related attributes are accessed.
+
+Contributors
+------------
+- tangkong

pcdsdevices/interface.py

@@ -4,6 +4,7 @@
 import functools
 import logging
 import numbers
+import os
 import re
 import shutil
 import signal
@@ -747,6 +748,21 @@ class FltMvInterface(MvInterface):
     def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self.presets = Presets(self)
+        self._presets_initialized = False
+
+    def __dir__(self):
+        # Initialize presets if tab-completion requested.
+        if self.presets.sync_needed():
+            self.presets.sync()
+
+        return super().__dir__()
+
+    def __getattribute__(self, name: str):
+        if (any((name.startswith(prefix)) for prefix in ['mv_', 'wm_', 'umv_'])
+                and self.presets.sync_needed()):
+            self.presets.sync()
+
+        return super().__getattribute__(name)
 
     def wm(self):
         pos = super().wm()
@@ -899,7 +915,7 @@ def set_position(self, position):
         self.set_current_position(position)
 
 
-def setup_preset_paths(**paths):
+def setup_preset_paths(defer_loading: bool = False, **paths):
     """
     Prepare the :class:`Presets` class.
 
@@ -911,13 +927,15 @@ def setup_preset_paths(**paths):
         A mapping from type of preset to destination path. These will be
         directories that contain the yaml files that define the preset
         positions.
+    defer_loading : bool, by default False
+        (Optional) "defer_loading": bool, whether or not to defer the loading
+        of preset files until the first tab completion
     """
-
     Presets._paths = {}
     for k, v in paths.items():
         Presets._paths[k] = Path(v)
     for preset in Presets._registry:
-        preset.sync()
+        preset.sync(defer_loading=defer_loading)
 
 
 class Presets:
@@ -953,9 +971,10 @@ def __init__(self, device):
         self._fd = None
         self._registry.add(self)
         self.name = device.name + '_presets'
+        self._mtimes = {}
         self.sync()
 
-    def _path(self, preset_type):
+    def _path(self, preset_type) -> Path:
         """Utility function to get the preset file :class:`~pathlib.Path`."""
         path = self._paths[preset_type] / (self._device.name + '.yml')
         logger.debug('select presets path %s', path)
@@ -1079,22 +1098,26 @@ def _update(self, preset_type, name, value=None, comment=None,
         except BlockingIOError:
             self._log_flock_error()
 
-    def sync(self):
+    def sync(self, defer_loading: bool = False):
         """Synchronize the presets with the database."""
         logger.debug('call %s presets.sync()', self._device.name)
         self._remove_methods()
         self._cache = {}
-        logger.debug('filling %s cache', self.name)
-        for preset_type in self._paths.keys():
-            path = self._path(preset_type)
-            if path.exists():
-                try:
-                    self._cache[preset_type] = self._read(preset_type)
-                except BlockingIOError:
-                    self._log_flock_error()
-            else:
-                logger.debug('No %s preset file for %s',
-                             preset_type, self._device.name)
+        self._mtimes = {}
+        # only consult files if requested
+        if not defer_loading:
+            logger.debug('filling %s cache', self.name)
+            for preset_type in self._paths.keys():
+                path = self._path(preset_type)
+                if path.exists():
+                    self._mtimes[preset_type] = os.path.getmtime(path)
+                    try:
+                        self._cache[preset_type] = self._read(preset_type)
+                    except BlockingIOError:
+                        self._log_flock_error()
+                else:
+                    logger.debug('No %s preset file for %s',
+                                 preset_type, self._device.name)
         self._create_methods()
 
     def _log_flock_error(self):
@@ -1140,6 +1163,7 @@ def _register_method(self, obj, method_name, method):
         self._methods.append((obj, method_name))
         setattr(obj, method_name, MethodType(method, obj))
         if hasattr(obj, '_tab'):
+            # obj._tab: TabCompletionHelperInstance
             obj._tab.add(method_name)
 
     def _make_add(self, preset_type):
@@ -1304,6 +1328,16 @@ def state(self):
                     closest = diff
         return state
 
+    def sync_needed(self) -> bool:
+        """True if this preset has fallen out of sync with backing files"""
+        curr_mtimes = {}
+        for preset_type in self._paths.keys():
+            preset_path = self._path(preset_type)
+            if preset_path.exists():
+                curr_mtimes[preset_type] = os.path.getmtime(preset_path)
+
+        return not curr_mtimes == self._mtimes
+
 
 class PresetPosition:
     """

pcdsdevices/tests/sim_fast_presets/sim_fast.yml

@@ -0,0 +1,17 @@
+directbeam:
+  active: true
+  history:
+    02 Nov 2022 19:02:48: '    8.4999'
+  value: 9.499946289065
+in:
+  active: true
+  history:
+    28 Feb 2024 14:59:31: '    0.0000'
+    28 Feb 2024 15:20:01: '    3.3000'
+  value: 3.3
+out:
+  active: true
+  history:
+    28 Feb 2024 15:00:08: '   15.0000'
+    28 Feb 2024 15:19:54: '   13.1800'
+  value: 13.18

pcdsdevices/tests/test_interface.py

@@ -3,6 +3,7 @@
 import sys
 import threading
 import time
+from pathlib import Path
 
 import ophyd
 import pytest
@@ -31,6 +32,15 @@ def fast_motor():
     return FastMotor(name='sim_fast')
 
 
+@pytest.fixture(scope="function")
+def deferred_fast_motor_presets():
+    """deferred loading fast_motor presets, backed by a file but unloaded"""
+    setup_preset_paths(defer_loading=True,
+                       hutch=Path(__file__).parent / 'sim_fast_presets')
+    yield
+    setup_preset_paths()
+
+
 @pytest.mark.timeout(5)
 def test_mv(fast_motor):
     logger.debug('test_mv')
@@ -101,7 +111,7 @@ def inner_test():
     sys.platform in ("win32", "darwin"),
     reason="Fails on Windows, no fcntl and different signal handling",
 )
-def test_presets(presets, fast_motor):
+def test_presets(presets, fast_motor: FastMotor):
     logger.debug('test_presets')
 
     fast_motor.mv(4, wait=True)
@@ -181,7 +191,7 @@ def block_file(path, lock):
     assert hasattr(fast_motor, 'mv_sample')
 
 
-def test_presets_type(presets, fast_motor):
+def test_presets_type(presets, fast_motor: FastMotor):
     logger.debug('test_presets_type')
     # Mess up the input types, fail before opening the file
 
@@ -191,6 +201,55 @@ def test_presets_type(presets, fast_motor):
         fast_motor.presets.add_user(234234, 'cats')
 
 
+def test_presets_desync(presets, fast_motor: FastMotor):
+    assert not fast_motor.presets.sync_needed()
+
+    fast_motor.mv(4, wait=True)
+    fast_motor.presets.add_hutch('four', comment='four!')
+
+    assert not fast_motor.presets.sync_needed()
+
+    # modify preset from other object with the same, to force collision
+    fast_motor2 = FastMotor(name='sim_fast')
+    fast_motor2.mv(5, wait=True)
+    fast_motor2.presets.positions.four.update_pos()
+
+    # in-memory python objects have different preset positions
+    assert fast_motor.presets.positions.four.pos == 4
+    assert fast_motor2.presets.positions.four.pos == 5
+
+    # but point to the same file
+    assert fast_motor.presets._path("hutch") == fast_motor2.presets._path("hutch")
+
+    # original object needs a sync, but second does not
+    assert fast_motor.presets.sync_needed()
+    assert not fast_motor2.presets.sync_needed()
+
+
+def test_presets_tab_init(fast_motor: FastMotor, deferred_fast_motor_presets):
+    # deferred_fast_motor_preset must come last,
+    # to clear cache after motor is created (and sync-ed at init)
+    assert fast_motor.presets.sync_needed()
+    fast_motor.__dir__()  # mimic tab completion request
+    assert not fast_motor.presets.sync_needed()
+
+
+@pytest.mark.parametrize("attr,", [
+    "wm_dne", "wm_in", "mv_dne", "mv_in", "umv_dne, umv_in"
+])
+def test_presets_getattribute_init(
+    fast_motor: FastMotor, attr: str, deferred_fast_motor_presets
+):
+    # deferred_fast_motor_preset must come last,
+    # to clear cache after motor is created (and sync-ed at init)
+    assert fast_motor.presets.sync_needed()
+    try:
+        getattr(fast_motor, attr)  # mimic completion request
+    except AttributeError:
+        pass
+    assert not fast_motor.presets.sync_needed()
+
+
 def test_engineering_mode():
     logger.debug('test_engineering_mode')
     set_engineering_mode(False)