Skip to content

Commit

Permalink
Copy cache results in some instances and rename to lowercase.
Browse files Browse the repository at this point in the history
Copy (shallow) to prevent lists from being altered, because those are references in the cache.

part of CURA-11761
  • Loading branch information
rburema committed Sep 11, 2024
1 parent dae5d6c commit eb698dc
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 36 deletions.
13 changes: 11 additions & 2 deletions UM/Decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ def timed(*args, **kw):


class CachedMemberFunctions:
"""Helper class to handle instance-cache w.r.t. results of member-functions decorated with '@cachePerInstance'."""
"""Helper class to handle instance-cache w.r.t. results of member-functions decorated with '@cache_per_instance'."""

__cache = {}

Expand Down Expand Up @@ -171,7 +171,16 @@ def callMemberFunction(cls, instance, function, *args, **kwargs):
return cls.__cache[instance][function](*args)


def cachePerInstance(function):
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
22 changes: 11 additions & 11 deletions UM/Settings/ContainerStack.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import UM.FlameProfiler

from UM.ConfigurationErrorMessage import ConfigurationErrorMessage
from UM.Decorators import CachedMemberFunctions, cachePerInstance
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
Expand Down Expand Up @@ -182,7 +182,7 @@ def setMetaData(self, meta_data: Dict[str, Any]) -> None:
metaDataChanged = pyqtSignal(QObject)
metaData = pyqtProperty("QVariantMap", fget = getMetaData, fset = setMetaData, notify = metaDataChanged)

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

Expand Down Expand Up @@ -229,7 +229,7 @@ def setDirty(self, dirty: bool) -> None:

containersChanged = Signal()

@cachePerInstance
@cache_per_instance
def getProperty(self, key: str, property_name: str, context: Optional[PropertyEvaluationContext] = None) -> Any:
""":copydoc ContainerInterface::getProperty
Expand All @@ -256,7 +256,7 @@ def getProperty(self, key: str, property_name: str, context: Optional[PropertyEv

return value

@cachePerInstance
@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.
Expand Down Expand Up @@ -306,7 +306,7 @@ def getRawProperty(self, key: str, property_name: str, *, context: Optional[Prop
else:
return None

@cachePerInstance
@cache_per_instance
def hasProperty(self, key: str, property_name: str) -> bool:
""":copydoc ContainerInterface::hasProperty
Expand Down Expand Up @@ -509,7 +509,7 @@ def deserializeMetadata(cls, serialized: str, container_id: str) -> List[Dict[st

return [metadata]

@cachePerInstance
@cache_per_instance_copy_result
def getAllKeys(self) -> Set[str]:
"""Get all keys known to this container stack.
Expand All @@ -527,7 +527,7 @@ def getAllKeys(self) -> Set[str]:
keys |= self._next_stack.getAllKeys()
return keys

@cachePerInstance
@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
Expand All @@ -545,7 +545,7 @@ def getAllKeysWithUserState(self) -> Set[str]:

return settings

@cachePerInstance
@cache_per_instance
def getContainers(self) -> List[ContainerInterface]:
"""Get a list of all containers in this stack.
Expand Down Expand Up @@ -616,7 +616,7 @@ def setPath(self, path: str) -> None:
CachedMemberFunctions.clearInstanceCache(self)
self._path = path

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

Expand Down Expand Up @@ -789,7 +789,7 @@ def sendPostponedEmits(self) -> None:
signal.emit(signal_arg)

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

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

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

Expand Down
14 changes: 7 additions & 7 deletions UM/Settings/InstanceContainer.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
from PyQt6.QtCore import QObject, pyqtProperty, pyqtSignal
from PyQt6.QtQml import QQmlEngine #To take ownership of this class ourselves.

from UM.Decorators import CachedMemberFunctions, cachePerInstance
from UM.Decorators import CachedMemberFunctions, cache_per_instance, cache_per_instance_copy_result
from UM.FastConfigParser import FastConfigParser
from UM.Trust import Trust
from UM.Decorators import override
Expand Down Expand Up @@ -308,7 +308,7 @@ def setDirty(self, dirty: bool) -> None:
CachedMemberFunctions.clearInstanceCache(self)
self._dirty = dirty

@cachePerInstance
@cache_per_instance
def getProperty(self, key: str, property_name: str, context: PropertyEvaluationContext = None) -> Any:
""":copydoc ContainerInterface::getProperty
Expand All @@ -327,7 +327,7 @@ def getProperty(self, key: str, property_name: str, context: PropertyEvaluationC

return None

@cachePerInstance
@cache_per_instance
def hasProperty(self, key: str, property_name: str) -> bool:
""":copydoc ContainerInterface::hasProperty
Expand Down Expand Up @@ -435,7 +435,7 @@ def clear(self) -> None:
self.removeInstance(key, postpone_emit=True)
self.sendPostponedEmits()

@cachePerInstance
@cache_per_instance_copy_result
def getAllKeys(self) -> Set[str]:
"""Get all the keys of the instances of this container
:returns: list of keys
Expand Down Expand Up @@ -702,7 +702,7 @@ def findInstances(self, **kwargs: Any) -> List[SettingInstance]:

return result

@cachePerInstance
@cache_per_instance
def getInstance(self, key: str) -> Optional[SettingInstance]:
"""Get an instance by key"""

Expand Down Expand Up @@ -776,7 +776,7 @@ def update(self) -> None:
self.propertyChanged.emit(key, property_name)
self._dirty = True

@cachePerInstance
@cache_per_instance
def getDefinition(self) -> DefinitionContainerInterface:
"""Get the DefinitionContainer used for new instance creation."""

Expand Down Expand Up @@ -831,7 +831,7 @@ def sendPostponedEmits(self) -> None:
signal, signal_arg = self._postponed_emits.pop(0)
signal.emit(*signal_arg)

@cachePerInstance
@cache_per_instance_copy_result
def getAllKeysWithUserState(self)-> Set[str]:
"""Get the keys of all the setting having a User state"""
self._instantiateCachedValues()
Expand Down
4 changes: 2 additions & 2 deletions UM/Settings/Models/SettingPropertyProvider.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from PyQt6.QtQml import QQmlPropertyMap
from UM.FlameProfiler import pyqtSlot

from UM.Decorators import CachedMemberFunctions, cachePerInstance
from UM.Decorators import CachedMemberFunctions, cache_per_instance
from UM.Logger import Logger
from UM.Application import Application
from UM.Settings.ContainerStack import ContainerStack
Expand Down Expand Up @@ -433,7 +433,7 @@ def _updateStackLevels(self) -> None:
self._stack_levels = levels
self.stackLevelChanged.emit()

@cachePerInstance
@cache_per_instance
def _getPropertyValue(self, property_name):
# Use the evaluation context to skip certain containers
context = PropertyEvaluationContext(self._stack)
Expand Down
18 changes: 9 additions & 9 deletions UM/Settings/SettingDefinition.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import re
from typing import Any, List, Dict, Callable, Match, Set, Union, Optional

from UM.Decorators import CachedMemberFunctions, cachePerInstance
from UM.Decorators import CachedMemberFunctions, cache_per_instance, cache_per_instance_copy_result
from UM.Logger import Logger
from UM.Settings.Interfaces import DefinitionContainerInterface
from UM.i18n import i18nCatalog
Expand Down Expand Up @@ -238,7 +238,7 @@ def relations(self) -> List["SettingRelation"]:

return self._relations

@cachePerInstance
@cache_per_instance
def relationsAsFrozenSet(self) -> frozenset["SettingRelation"]:
"""A frozen set of SettingRelation objects of this setting.
Expand All @@ -255,7 +255,7 @@ def serialize(self) -> str:

pass

@cachePerInstance
@cache_per_instance_copy_result
def getAllKeys(self) -> Set[str]:
"""Gets the key of this setting definition and of all its descendants.
Expand Down Expand Up @@ -301,7 +301,7 @@ def deserialize(self, serialized: Union[str, Dict[str, Any]]) -> None:
parsed = json.loads(serialized, object_pairs_hook=collections.OrderedDict)
self._deserialize_dict(parsed)

@cachePerInstance
@cache_per_instance
def getChild(self, key: str) -> Optional["SettingDefinition"]:
"""Get a child by key
Expand All @@ -323,7 +323,7 @@ def getChild(self, key: str) -> Optional["SettingDefinition"]:

return None

@cachePerInstance
@cache_per_instance
def _matches1l8nProperty(self, property_name: str, value: Any, catalog) -> bool:
try:
property_value = getattr(self, property_name)
Expand Down Expand Up @@ -446,7 +446,7 @@ def findDefinitions(self, **kwargs: Any) -> List["SettingDefinition"]:

return definitions

@cachePerInstance
@cache_per_instance
def isAncestor(self, key: str) -> bool:
"""Check whether a certain setting is an ancestor of this definition.
Expand All @@ -460,7 +460,7 @@ def isAncestor(self, key: str) -> bool:

return key in self.__ancestors

@cachePerInstance
@cache_per_instance
def isDescendant(self, key: str) -> bool:
"""Check whether a certain setting is a descendant of this definition.
Expand All @@ -474,7 +474,7 @@ def isDescendant(self, key: str) -> bool:

return key in self.__descendants

@cachePerInstance
@cache_per_instance_copy_result
def getAncestors(self) -> Set[str]:
"""Get a set of keys representing the setting's ancestors."""

Expand Down Expand Up @@ -737,7 +737,7 @@ def _deserialize_dict(self, serialized: Dict[str, Any]) -> None:
self.__ancestors = self._updateAncestors()
self.__descendants = self._updateDescendants()

@cachePerInstance
@cache_per_instance_copy_result
def _updateAncestors(self) -> Set[str]:
result = set() # type: Set[str]

Expand Down
6 changes: 3 additions & 3 deletions UM/Settings/SettingInstance.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import os
from typing import Any, cast, Dict, Iterable, List, Optional, Set, TYPE_CHECKING

from UM.Decorators import CachedMemberFunctions, cachePerInstance
from UM.Decorators import CachedMemberFunctions, cache_per_instance
from UM.Settings.Interfaces import ContainerInterface
from UM.Signal import Signal, signalemitter
from UM.Logger import Logger
Expand Down Expand Up @@ -92,7 +92,7 @@ def __init__(self, definition: SettingDefinition, container: ContainerInterface,

self.__property_values = {} # type: Dict[str, Any]

@cachePerInstance
@cache_per_instance
def getPropertyNames(self) -> Iterable[str]:
"""Get a list of all supported property names"""

Expand Down Expand Up @@ -136,7 +136,7 @@ def __hash__(self) -> int:
def __ne__(self, other: object) -> bool:
return not (self == other)

@cachePerInstance
@cache_per_instance
def __getattr__(self, name: str) -> Any:
if name == "_SettingInstance__property_values":
# Prevent infinite recursion when __property_values is not set.
Expand Down
14 changes: 12 additions & 2 deletions tests/TestDecorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

import pytest

from UM.Decorators import interface, CachedMemberFunctions, cachePerInstance
from UM.Decorators import interface, CachedMemberFunctions, cache_per_instance, cache_per_instance_copy_result

def test_interface():
def declare_interface():
Expand Down Expand Up @@ -111,11 +111,16 @@ class SomeClass:
def __init__(self):
self._map = {}

@cachePerInstance
@cache_per_instance
def getThing(self, a):
bigDeal()
return self._map.get(a, None)

@cache_per_instance_copy_result
def getList(self):
bigDeal()
return [234, 456, 789]

def setThing(self, a, b):
CachedMemberFunctions.clearInstanceCache(self)
self._map[a] = b
Expand Down Expand Up @@ -145,3 +150,8 @@ def setThing(self, a, b):
assert len(CachedMemberFunctions._CachedMemberFunctions__cache) == 2
CachedMemberFunctions.deleteInstanceCache(instance)
assert len(CachedMemberFunctions._CachedMemberFunctions__cache) == 1

lizt = other.getList()
assert lizt == [234, 456, 789]
lizt.append(111)
assert other.getList() == [234, 456, 789]

0 comments on commit eb698dc

Please sign in to comment.