Skip to content

Commit 49d038d

Browse files
authored
refactor(hardware, shared-data, api): add bounds to unbounded lru caches (#19110)
# Overview Unbounded caches have the potential for causing memory leaks. From a code audit standpoint, it is difficult to verify whether or not a particular cache is responsible for a leak without fulling understanding how the caching API interacts with its callees and the parameters passed to the API. Yes, this is entirely a paranoia PR, but it seems like a reasonable ask to bound caches so that: - Any upstream changes to a cached API don't cause memory issues. Bounding caches eliminates a bug vector. - Auditing LRU caches in the future is a bit easier. - Future API authors, when adding a cache to a function, see that the rest of our codebase always defines a cache size and therefore do so themselves (or that is hopefully more likely, at least). What do we think?
1 parent 68b7a81 commit 49d038d

File tree

5 files changed

+10
-10
lines changed

5 files changed

+10
-10
lines changed

api/src/opentrons/legacy_commands/publisher.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ def publish_context(broker: LegacyBroker, command: CommandPayload) -> Iterator[N
124124
_do_publish(broker=broker, message_id=message_id, command=command, when="after")
125125

126126

127-
@functools.lru_cache(maxsize=None)
127+
@functools.lru_cache(maxsize=100)
128128
def _inspect_signature(func: Callable[..., Any]) -> inspect.Signature:
129129
"""Inspect function signatures, memoized because it is called very often."""
130130
return inspect.signature(func)

hardware/opentrons_hardware/firmware_bindings/messages/binary_message_definitions.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -449,7 +449,7 @@ class GetDeckLightResponse(utils.BinarySerializable):
449449
]
450450

451451

452-
@lru_cache(maxsize=None)
452+
@lru_cache(maxsize=100)
453453
def get_binary_definition(
454454
message_id: BinaryMessageId,
455455
) -> Optional[Type[BinaryMessageDefinition]]:

hardware/opentrons_hardware/firmware_bindings/messages/messages.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,7 +118,7 @@
118118
]
119119

120120

121-
@lru_cache(maxsize=None)
121+
@lru_cache(maxsize=100)
122122
def get_definition(message_id: MessageId) -> Optional[Type[MessageDefinition]]:
123123
"""Get the message type for a message id.
124124

shared-data/python/opentrons_shared_data/pipette/__init__.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ def model_config() -> PipetteModelSpecs:
3838
return copy.deepcopy(_model_config())
3939

4040

41-
@lru_cache(maxsize=None)
41+
@lru_cache(maxsize=1)
4242
def _model_config() -> PipetteModelSpecs:
4343
return json.loads(
4444
load_shared_data("pipette/definitions/1/pipetteModelSpecs.json") or "{}"
@@ -50,7 +50,7 @@ def name_config() -> PipetteNameSpecs:
5050
return _name_config()
5151

5252

53-
@lru_cache(maxsize=None)
53+
@lru_cache(maxsize=1)
5454
def _name_config() -> PipetteNameSpecs:
5555
return json.loads(
5656
load_shared_data("pipette/definitions/1/pipetteNameSpecs.json") or "{}"
@@ -73,7 +73,7 @@ def fuse_specs(
7373
return copy.deepcopy(_fuse_specs_cached(pipette_model, pipette_name))
7474

7575

76-
@lru_cache(maxsize=None)
76+
@lru_cache(maxsize=10)
7777
def _fuse_specs_cached(
7878
pipette_model: PipetteModel, pipette_name: Optional[PipetteName] = None
7979
) -> PipetteFusedSpec:

shared-data/python/opentrons_shared_data/pipette/load_data.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ def _get_configuration_dictionary(
6666
return json.loads(load_shared_data(config_path))
6767

6868

69-
@lru_cache(maxsize=None)
69+
@lru_cache(maxsize=10)
7070
def _geometry(
7171
channels: PipetteChannelType,
7272
model: PipetteModelType,
@@ -76,7 +76,7 @@ def _geometry(
7676
return _get_configuration_dictionary("geometry", channels, model, version, oem)
7777

7878

79-
@lru_cache(maxsize=None)
79+
@lru_cache(maxsize=10)
8080
def _liquid(
8181
channels: PipetteChannelType,
8282
model: PipetteModelType,
@@ -95,7 +95,7 @@ def _liquid(
9595
return liquid_dict
9696

9797

98-
@lru_cache(maxsize=None)
98+
@lru_cache(maxsize=10)
9999
def _physical(
100100
channels: PipetteChannelType,
101101
model: PipetteModelType,
@@ -111,7 +111,7 @@ def _dirs_in(path: Path) -> Iterator[Path]:
111111
yield child
112112

113113

114-
@lru_cache(maxsize=None)
114+
@lru_cache(maxsize=1)
115115
def load_serial_lookup_table() -> Dict[str, str]:
116116
"""Load a serial abbreviation lookup table mapped to model name."""
117117
config_path = get_shared_data_root() / "pipette" / "definitions" / "2" / "general"

0 commit comments

Comments
 (0)