Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 22 commits into from
Sep 20, 2024
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
a9e546b
Add 'per-instance' cacheing of member-methods (results) functionality
rburema Aug 28, 2024
5ddb858
Use the newly introduced caching wrapper for container-stacks.
rburema Aug 28, 2024
cd28092
'Re-enable' garbage collection on objects with cached members.
rburema Aug 28, 2024
649c92c
Make tests work on Windows again.
rburema Aug 28, 2024
bd5db05
Fix unit test.
rburema Aug 28, 2024
2852205
Fix cache filling/clearing errors caught by unit-tests.
rburema Aug 28, 2024
fe3543a
Cache function-results from Settings-instances.
rburema Aug 28, 2024
cd1562f
Cache function-results from instance containers.
rburema Aug 28, 2024
7527d0d
Small update documentation.
rburema Aug 28, 2024
69a60b8
Workaround for sets not hashing (use frozenset) and cache definition.
rburema Sep 3, 2024
610854b
Cache results for _getPropertyValue.
rburema Sep 3, 2024
4afc270
Refactor instance-methods-cache class.
rburema Sep 3, 2024
032de4b
Clean up cache (more) when objects get deleted.
rburema Sep 3, 2024
0b6eb8b
Class-member-cache: Add tests and documentation.
rburema Sep 3, 2024
5106ab3
Remove cache before cache unit-test begins.
rburema Sep 3, 2024
e8fb0c0
Also do a 'clear-instance' in the other branch.
rburema Sep 10, 2024
dae5d6c
Accumulate by return value instead, since a frozen-set can't grow.
rburema Sep 10, 2024
eb698dc
Copy cache results in some instances and rename to lowercase.
rburema Sep 11, 2024
35ec22f
Refactor: Called in both if-branches, so consolidate.
rburema Sep 11, 2024
33c8958
Also call deletion-handler or parent if added.
rburema Sep 11, 2024
ddd53ff
Add thread-lock to cache to prevent race-condition(s).
rburema Sep 18, 2024
2be76be
Merge branch 'main' into CURA-11761_container_stack_cache
HellAholic Sep 19, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 39 additions & 2 deletions UM/Decorators.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
# 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

Expand Down Expand Up @@ -137,4 +138,40 @@ 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 '@cachePerInstance'."""

__cache = {}

@classmethod
def clearInstanceCache(cls, instance):
"""Clear all the cache-entries for the specified instance."""
cls.__cache[instance] = {}

@classmethod
def deleteInstanceCache(cls, instance):
"""Completely delete the entry of the specified 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)
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))
return cls.__cache[instance][function](*args)


def cachePerInstance(function):
def wrapper(instance, *args, **kwargs):
return CachedMemberFunctions.callMemberFunction(instance, function, *args, **kwargs)
return wrapper
58 changes: 51 additions & 7 deletions UM/Settings/ContainerStack.py
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -12,6 +13,7 @@
import UM.FlameProfiler

from UM.ConfigurationErrorMessage import ConfigurationErrorMessage
from UM.Decorators import CachedMemberFunctions, cachePerInstance
from UM.Signal import Signal, signalemitter
from UM.PluginObject import PluginObject
from UM.MimeTypeDatabase import MimeTypeDatabase, MimeType
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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()
Expand All @@ -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()

Expand All @@ -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.
Expand All @@ -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
"""

@cachePerInstance
def _getRawMetaDataEntry(self, entry: str) -> Any:
value = self._metadata.get(entry, None)

if value is None:
Expand All @@ -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()

@cachePerInstance
def getProperty(self, key: str, property_name: str, context: Optional[PropertyEvaluationContext] = None) -> Any:
""":copydoc ContainerInterface::getProperty

Expand All @@ -240,6 +256,7 @@ def getProperty(self, key: str, property_name: str, context: Optional[PropertyEv

return value

@cachePerInstance
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.

Expand Down Expand Up @@ -289,6 +306,7 @@ def getRawProperty(self, key: str, property_name: str, *, context: Optional[Prop
else:
return None

@cachePerInstance
def hasProperty(self, key: str, property_name: str) -> bool:
""":copydoc ContainerInterface::hasProperty

Expand Down Expand Up @@ -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.

Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -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)
Expand Down Expand Up @@ -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.

Expand Down Expand Up @@ -485,6 +509,7 @@ def deserializeMetadata(cls, serialized: str, container_id: str) -> List[Dict[st

return [metadata]

@cachePerInstance
def getAllKeys(self) -> Set[str]:
"""Get all keys known to this container stack.

Expand All @@ -502,6 +527,7 @@ def getAllKeys(self) -> Set[str]:
keys |= self._next_stack.getAllKeys()
return keys

@cachePerInstance
def getAllKeysWithUserState(self) -> Set[str]:
"""Get a subset of all the settings keys in all the containers having a User state

Expand All @@ -519,6 +545,7 @@ def getAllKeysWithUserState(self) -> Set[str]:

return settings

@cachePerInstance
def getContainers(self) -> List[ContainerInterface]:
"""Get a list of all containers in this stack.

Expand Down Expand Up @@ -586,8 +613,10 @@ def setPath(self, path: str) -> None:
Reimplemented from ContainerInterface
"""

CachedMemberFunctions.clearInstanceCache(self)
self._path = path

@cachePerInstance
def getSettingDefinition(self, key: str) -> Optional[SettingDefinition]:
"""Get the SettingDefinition object for a specified key"""

Expand All @@ -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]:
rburema marked this conversation as resolved.
Show resolved Hide resolved
"""Find a container matching certain criteria.

Expand Down Expand Up @@ -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:
Expand All @@ -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)
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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)
Expand All @@ -754,6 +789,7 @@ def sendPostponedEmits(self) -> None:
signal.emit(signal_arg)

@UM.FlameProfiler.profile
@cachePerInstance
def hasErrors(self) -> bool:
"""Check if the container stack has errors"""

Expand All @@ -776,6 +812,7 @@ def hasErrors(self) -> bool:
return False

@UM.FlameProfiler.profile
@cachePerInstance
def getErrorKeys(self) -> List[str]:
"""Get all the keys that are in an error state in this stack"""

Expand Down Expand Up @@ -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()

Expand Down Expand Up @@ -832,6 +871,11 @@ def __str__(self) -> str:
def __repr__(self) -> str:
return str(self)

def __del__(self) -> None:
# None of the 'parents' seem to have __del__, so OK not to call `super.del` here.
CachedMemberFunctions.deleteInstanceCache(self)
wawanbreton marked this conversation as resolved.
Show resolved Hide resolved


_containerRegistry = ContainerRegistryInterface() # type: ContainerRegistryInterface


Expand Down
Loading
Loading