Merge pull request #966 from Ultimaker/CURA-11761_container_stack_cache

[CURA-11761] Cache results of class-member methods

UM/Decorators.py

@@ -1,9 +1,11 @@
-# Copyright (c) 2018 Ultimaker B.V.
+# Copyright (c) 2024 UltiMaker
 # Uranium is released under the terms of the LGPLv3 or higher.
 
 import copy
+from functools import lru_cache, partial
 import warnings
 import inspect
+from threading import Lock
 
 from UM.Logger import Logger
 from typing import Callable, Any
@@ -137,4 +139,61 @@ def timed(*args, **kw):
             print("Function %r took %2.2f ms" % (method.__name__, (te - ts) * 1000))
         return result
 
-    return timed
+    return timed
+
+
+class CachedMemberFunctions:
+    """Helper class to handle instance-cache w.r.t. results of member-functions decorated with '@cache_per_instance'."""
+
+    __cache = {}
+    __locks = {}
+
+    @classmethod
+    def _getInstanceLock(cls, instance):
+        if instance not in cls.__locks:
+            cls.__locks[instance] = Lock()
+        return cls.__locks[instance]
+
+    @classmethod
+    def clearInstanceCache(cls, instance):
+        """Clear all the cache-entries for the specified instance."""
+        with cls._getInstanceLock(instance):
+            cls.__cache[instance] = {}
+
+    @classmethod
+    def deleteInstanceCache(cls, instance):
+        """Completely delete the entry of the specified instance."""
+        with cls._getInstanceLock(instance):
+            if instance in cls.__cache:
+                del cls.__cache[instance]
+
+    @classmethod
+    def callMemberFunction(cls, instance, function, *args, **kwargs):
+        """Call the specified member function, make use of (results) cache if available, and create if not."""
+        if kwargs is not None and len(kwargs) > 0:
+            # NOTE The `lru_cache` can't handle keyword-arguments (because it's a dict).
+            # We could make a frozendict, but that's probably a lot more hassle than it's worth, so just call normally.
+            return function(instance, *args, **kwargs)
+        with cls._getInstanceLock(instance):
+            if instance not in cls.__cache:
+                cls.__cache[instance] = {}
+            if function not in cls.__cache[instance]:
+                cls.__cache[instance][function] = lru_cache()(partial(function, instance))
+            func = cls.__cache[instance][function]
+        # Need to call the function outside the locked part, as it may introduce a race-condition otherwise.
+        return func(*args)
+
+
+def cache_per_instance(function):
+    def wrapper(instance, *args, **kwargs):
+        return CachedMemberFunctions.callMemberFunction(instance, function, *args, **kwargs)
+    return wrapper
+
+
+def cache_per_instance_copy_result(function):
+    def wrapper(instance, *args, **kwargs):
+        result = CachedMemberFunctions.callMemberFunction(instance, function, *args, **kwargs)
+        if hasattr(result, "copy"):
+            return result.copy()
+        return copy.copy(result)
+    return wrapper

UM/Settings/ContainerStack.py

@@ -1,8 +1,9 @@
-# Copyright (c) 2022 Ultimaker B.V.
+# Copyright (c) 2024 UltiMaker
 # Uranium is released under the terms of the LGPLv3 or higher.
 
 import configparser
 import io
+from functools import lru_cache
 from typing import Any, cast, Dict, List, Optional, Set, Tuple
 
 from PyQt6.QtCore import QObject, pyqtProperty, pyqtSignal
@@ -12,6 +13,7 @@
 import UM.FlameProfiler
 
 from UM.ConfigurationErrorMessage import ConfigurationErrorMessage
+from UM.Decorators import CachedMemberFunctions, cache_per_instance, cache_per_instance_copy_result
 from UM.Signal import Signal, signalemitter
 from UM.PluginObject import PluginObject
 from UM.MimeTypeDatabase import MimeTypeDatabase, MimeType
@@ -94,6 +96,7 @@ def __getstate__(self) -> Dict[str, Any]:
     def __setstate__(self, state: Dict[str, Any]) -> None:
         """For pickle support"""
 
+        CachedMemberFunctions.clearInstanceCache(self)
         self.__dict__.update(state)
 
     def getId(self) -> str:
@@ -121,6 +124,7 @@ def setName(self, name: str) -> None:
         """
 
         if name != self.getName():
+            CachedMemberFunctions.clearInstanceCache(self)
             self._metadata["name"] = name
             self._dirty = True
             self.nameChanged.emit()
@@ -141,6 +145,7 @@ def isReadOnly(self) -> bool:
 
     def setReadOnly(self, read_only: bool) -> None:
         if read_only != self._read_only:
+            CachedMemberFunctions.clearInstanceCache(self)
             self._read_only = read_only
             self.readOnlyChanged.emit()
 
@@ -160,6 +165,7 @@ def setMetaData(self, meta_data: Dict[str, Any]) -> None:
 
         if meta_data == self.getMetaData():
             return #Unnecessary.
+        CachedMemberFunctions.clearInstanceCache(self)
 
         #We'll fill a temporary dictionary with all the required metadata and overwrite it with the new metadata.
         #This way it is ensured that at least the required metadata is still there.
@@ -176,12 +182,8 @@ def setMetaData(self, meta_data: Dict[str, Any]) -> None:
     metaDataChanged = pyqtSignal(QObject)
     metaData = pyqtProperty("QVariantMap", fget = getMetaData, fset = setMetaData, notify = metaDataChanged)
 
-    def getMetaDataEntry(self, entry: str, default: Any = None) -> Any:
-        """:copydoc ContainerInterface::getMetaDataEntry
-
-        Reimplemented from ContainerInterface
-        """
-
+    @cache_per_instance
+    def _getRawMetaDataEntry(self, entry: str) -> Any:
         value = self._metadata.get(entry, None)
 
         if value is None:
@@ -190,30 +192,44 @@ def getMetaDataEntry(self, entry: str, default: Any = None) -> Any:
                 if value is not None:
                     break
 
+        return value
+
+    # Note that '_getRawMetaDataEntry' is cached, not this one, because default may be a list and that's unhashable.
+    def getMetaDataEntry(self, entry: str, default: Any = None) -> Any:
+        """:copydoc ContainerInterface::getMetaDataEntry
+
+        Reimplemented from ContainerInterface
+        """
+
+        value = self._getRawMetaDataEntry(entry)
         if value is None:
             return default
         else:
             return value
 
     def setMetaDataEntry(self, key: str, value: Any) -> None:
         if key not in self._metadata or self._metadata[key] != value:
+            CachedMemberFunctions.clearInstanceCache(self)
             self._metadata[key] = value
             self._dirty = True
             self.metaDataChanged.emit(self)
 
     def removeMetaDataEntry(self, key: str) -> None:
         if key in self._metadata:
+            CachedMemberFunctions.clearInstanceCache(self)
             del self._metadata[key]
             self.metaDataChanged.emit(self)
 
     def isDirty(self) -> bool:
         return self._dirty
 
     def setDirty(self, dirty: bool) -> None:
+        CachedMemberFunctions.clearInstanceCache(self)
         self._dirty = dirty
 
     containersChanged = Signal()
 
+    @cache_per_instance
     def getProperty(self, key: str, property_name: str, context: Optional[PropertyEvaluationContext] = None) -> Any:
         """:copydoc ContainerInterface::getProperty
 
@@ -240,6 +256,7 @@ def getProperty(self, key: str, property_name: str, context: Optional[PropertyEv
 
         return value
 
+    @cache_per_instance
     def getRawProperty(self, key: str, property_name: str, *, context: Optional[PropertyEvaluationContext] = None, use_next: bool = True, skip_until_container: Optional[ContainerInterface] = None) -> Any:
         """Retrieve a property of a setting by key and property name.
 
@@ -289,6 +306,7 @@ def getRawProperty(self, key: str, property_name: str, *, context: Optional[Prop
         else:
             return None
 
+    @cache_per_instance
     def hasProperty(self, key: str, property_name: str) -> bool:
         """:copydoc ContainerInterface::hasProperty
 
@@ -346,6 +364,7 @@ def serialize(self, ignored_metadata_keys: Optional[Set[str]] = None) -> str:
         return stream.getvalue()
 
     @classmethod
+    @lru_cache
     def _readAndValidateSerialized(cls, serialized: str) -> configparser.ConfigParser:
         """Deserializes the given data and checks if the required fields are present.
 
@@ -362,6 +381,7 @@ def _readAndValidateSerialized(cls, serialized: str) -> configparser.ConfigParse
         return parser
 
     @classmethod
+    @lru_cache
     def getConfigurationTypeFromSerialized(cls, serialized: str) -> Optional[str]:
         configuration_type = None
         try:
@@ -374,6 +394,7 @@ def getConfigurationTypeFromSerialized(cls, serialized: str) -> Optional[str]:
         return configuration_type
 
     @classmethod
+    @lru_cache
     def getVersionFromSerialized(cls, serialized: str) -> Optional[int]:
         configuration_type = cls.getConfigurationTypeFromSerialized(serialized)
         if not configuration_type:
@@ -397,6 +418,8 @@ def deserialize(self, serialized: str, file_name: Optional[str] = None) -> str:
         TODO: Expand documentation here, include the fact that this should _not_ include all containers
         """
 
+        CachedMemberFunctions.clearInstanceCache(self)
+
         # Update the serialized data first
         serialized = super().deserialize(serialized, file_name)
         parser = self._readAndValidateSerialized(serialized)
@@ -452,6 +475,7 @@ def deserialize(self, serialized: str, file_name: Optional[str] = None) -> str:
         return serialized
 
     @classmethod
+    @lru_cache
     def deserializeMetadata(cls, serialized: str, container_id: str) -> List[Dict[str, Any]]:
         """Gets the metadata of a container stack from a serialised format.
 
@@ -485,6 +509,7 @@ def deserializeMetadata(cls, serialized: str, container_id: str) -> List[Dict[st
 
         return [metadata]
 
+    @cache_per_instance_copy_result
     def getAllKeys(self) -> Set[str]:
         """Get all keys known to this container stack.
 
@@ -502,6 +527,7 @@ def getAllKeys(self) -> Set[str]:
             keys |= self._next_stack.getAllKeys()
         return keys
 
+    @cache_per_instance_copy_result
     def getAllKeysWithUserState(self) -> Set[str]:
         """Get a subset of all the settings keys in all the containers having a User state
 
@@ -519,6 +545,7 @@ def getAllKeysWithUserState(self) -> Set[str]:
 
         return settings
 
+    @cache_per_instance
     def getContainers(self) -> List[ContainerInterface]:
         """Get a list of all containers in this stack.
 
@@ -586,8 +613,10 @@ def setPath(self, path: str) -> None:
         Reimplemented from ContainerInterface
         """
 
+        CachedMemberFunctions.clearInstanceCache(self)
         self._path = path
 
+    @cache_per_instance
     def getSettingDefinition(self, key: str) -> Optional[SettingDefinition]:
         """Get the SettingDefinition object for a specified key"""
 
@@ -605,6 +634,7 @@ def getSettingDefinition(self, key: str) -> Optional[SettingDefinition]:
             return None
 
     @UM.FlameProfiler.profile
+    #TODO: Find a way around keyword-arguments being passed as a dict, or live with not cacheing this particular func.
     def findContainer(self, criteria: Dict[str, Any] = None, container_type: type = None, **kwargs: Any) -> Optional[ContainerInterface]:
         """Find a container matching certain criteria.
 
@@ -648,6 +678,7 @@ def addContainer(self, container: ContainerInterface) -> None:
         :param container: The container to add to the stack.
         """
 
+        CachedMemberFunctions.clearInstanceCache(self)
         self.insertContainer(0, container)
 
     def insertContainer(self, index: int, container: ContainerInterface) -> None:
@@ -661,6 +692,7 @@ def insertContainer(self, index: int, container: ContainerInterface) -> None:
         if container is self:
             raise Exception("Unable to add stack to itself.")
 
+        CachedMemberFunctions.clearInstanceCache(self)
         container.propertyChanged.connect(self._collectPropertyChanges)
         self._containers.insert(index, container)
         self.containersChanged.emit(container)
@@ -682,6 +714,7 @@ def replaceContainer(self, index: int, container: ContainerInterface, postpone_e
         if container is self:
             raise Exception("Unable to replace container with ContainerStack (self) ")
 
+        CachedMemberFunctions.clearInstanceCache(self)
         self._containers[index].propertyChanged.disconnect(self._collectPropertyChanges)
         container.propertyChanged.connect(self._collectPropertyChanges)
         self._containers[index] = container
@@ -703,6 +736,7 @@ def removeContainer(self, index: int = 0) -> None:
         if index < 0:
             raise IndexError
         try:
+            CachedMemberFunctions.clearInstanceCache(self)
             container = self._containers[index]
             self._dirty = True
             container.propertyChanged.disconnect(self._collectPropertyChanges)
@@ -735,6 +769,7 @@ def setNextStack(self, stack: "ContainerStack", connect_signals: bool = True) ->
         if self._next_stack == stack:
             return
 
+        CachedMemberFunctions.clearInstanceCache(self)
         if self._next_stack:
             self._next_stack.propertyChanged.disconnect(self._collectPropertyChanges)
             self.containersChanged.disconnect(self._next_stack.containersChanged)
@@ -754,6 +789,7 @@ def sendPostponedEmits(self) -> None:
             signal.emit(signal_arg)
 
     @UM.FlameProfiler.profile
+    @cache_per_instance
     def hasErrors(self) -> bool:
         """Check if the container stack has errors"""
 
@@ -776,6 +812,7 @@ def hasErrors(self) -> bool:
         return False
 
     @UM.FlameProfiler.profile
+    @cache_per_instance_copy_result
     def getErrorKeys(self) -> List[str]:
         """Get all the keys that are in an error state in this stack"""
 
@@ -804,6 +841,8 @@ def getErrorKeys(self) -> List[str]:
     # In addition, it allows us to emit a single signal that reports all properties that
     # have changed.
     def _collectPropertyChanges(self, key: str, property_name: str) -> None:
+        CachedMemberFunctions.clearInstanceCache(self)
+
         if key not in self._property_changes:
             self._property_changes[key] = set()
 
@@ -832,6 +871,11 @@ def __str__(self) -> str:
     def __repr__(self) -> str:
         return str(self)
 
+    def __del__(self) -> None:
+        CachedMemberFunctions.deleteInstanceCache(self)
+        getattr(super(), "__del__", lambda s: None)(self)
+
+
 _containerRegistry = ContainerRegistryInterface()  # type: ContainerRegistryInterface