Skip to content

Commit ec28103

Browse files
berndverstBernd VerstCopilot
authored
Performance [P2]: Make entity method signature caching effective (#202)
* Make entity method signature caching effective _execute_entity_batch() constructed a new _EntityExecutor inside its per-operation loop, so the executor's _entity_method_cache was always cold and inspect.signature() was re-run for every single entity operation. The executor is now constructed once per batch and shares a bounded, thread-safe signature cache owned by the worker, so a given (entity type, operation) pair is reflected once for the lifetime of the worker rather than once per operation. The cache is bounded (_EntityMethodSignatureCache, 1024 entries, oldest evicted first) because entity types and operation names come from user code and are unbounded in principle. Writes are serialized under a lock so concurrently processed entity work items stay safe. Behavior is unchanged: the dispatch logic in _EntityExecutor.execute() is untouched, so the no-argument versus input-argument distinction and all error messages for unknown or invalid operations are preserved. _EntityExecutor's existing three-argument constructor still works; the shared cache is an optional fourth argument. Fixes #186 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2265f7b9-f8ad-4b50-a25d-19bd6f1ac96c * Test that a non-positive signature cache bound is clamped _EntityMethodSignatureCache clamps its bound with max(1, max_size), but nothing covered that clamp, so the PR description overstated the test coverage. Add regression tests for it. The clamp is not cosmetic. Without it a bound of 0 evicts each entry immediately after inserting it, so every lookup misses and the entity method reflection this cache exists to avoid runs on every operation again -- silently reintroducing the very problem this change fixes. A negative bound is worse: the eviction loop keeps popping past an already-empty dict and raises StopIteration. The new tests cover a zero and negative bound, that a clamped cache still behaves as a working size-one cache rather than a no-op, and that an executor handed a clamped cache still reflects only once. Verified red before green: with the clamp removed all five fail, including 'assert 3 == 1' on the reflection count. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2265f7b9-f8ad-4b50-a25d-19bd6f1ac96c --------- Co-authored-by: Bernd Verst <beverst@microsoft.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2265f7b9-f8ad-4b50-a25d-19bd6f1ac96c
1 parent 6a52e1d commit ec28103

2 files changed

Lines changed: 353 additions & 5 deletions

File tree

durabletask/worker.py

Lines changed: 62 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,9 @@ def __init__(
535535
data_converter: DataConverter | None = None,
536536
):
537537
self._registry = _Registry()
538+
# Shared by every entity batch this worker processes so that reflected
539+
# entity method signatures are computed once instead of per operation.
540+
self._entity_method_cache = _EntityMethodSignatureCache()
538541
self._host_address = (
539542
host_address if host_address else shared.get_default_host_address()
540543
)
@@ -1373,9 +1376,17 @@ def _execute_entity_batch(
13731376
raise RuntimeError(f"Invalid entity instance ID '{instance_id}' in entity operation request.")
13741377

13751378
results: list[pb.OperationResult] = []
1379+
# A single executor serves the whole batch and shares the worker's
1380+
# bounded signature cache, so entity method reflection is not repeated
1381+
# for every operation.
1382+
executor = _EntityExecutor(
1383+
self._registry,
1384+
self._logger,
1385+
self._data_converter,
1386+
entity_method_cache=self._entity_method_cache,
1387+
)
13761388
for operation in req.operations:
13771389
start_time = datetime.now(timezone.utc)
1378-
executor = _EntityExecutor(self._registry, self._logger, self._data_converter)
13791390

13801391
operation_result = None
13811392

@@ -3107,13 +3118,61 @@ def execute(
31073118
return encoded_output
31083119

31093120

3121+
class _EntityMethodSignatureCache:
3122+
"""Bounded, thread-safe cache of reflected entity method signatures.
3123+
3124+
Dispatching an operation on a class-based entity requires knowing whether
3125+
the target method declares a required parameter, which is answered with an
3126+
``inspect.signature()`` call. The answer depends only on the entity type
3127+
and the operation name, so it is cached here and shared across entity
3128+
batches (and therefore across the worker threads that process them
3129+
concurrently).
3130+
3131+
The cache is bounded because entity types and operation names originate
3132+
from user code and are unbounded in principle; once the limit is reached
3133+
the oldest entries are evicted instead of growing without limit.
3134+
"""
3135+
3136+
DEFAULT_MAX_SIZE = 1024
3137+
3138+
def __init__(self, max_size: int = DEFAULT_MAX_SIZE):
3139+
self._max_size = max(1, max_size)
3140+
self._lock = Lock()
3141+
self._entries: dict[tuple[type, str], bool] = {}
3142+
3143+
def get(self, key: tuple[type, str]) -> bool | None:
3144+
"""Returns the cached result for ``key``, or ``None`` when not cached."""
3145+
return self._entries.get(key)
3146+
3147+
def __setitem__(self, key: tuple[type, str], value: bool) -> None:
3148+
with self._lock:
3149+
self._entries[key] = value
3150+
# Dicts preserve insertion order, so the first key is the oldest.
3151+
while len(self._entries) > self._max_size:
3152+
self._entries.pop(next(iter(self._entries)), None)
3153+
3154+
def __contains__(self, key: object) -> bool:
3155+
return key in self._entries
3156+
3157+
def __len__(self) -> int:
3158+
return len(self._entries)
3159+
3160+
31103161
class _EntityExecutor:
31113162
def __init__(self, registry: _Registry, logger: logging.Logger,
3112-
data_converter: DataConverter):
3163+
data_converter: DataConverter,
3164+
entity_method_cache: _EntityMethodSignatureCache | None = None):
31133165
self._registry = registry
31143166
self._logger = logger
31153167
self._data_converter = data_converter
3116-
self._entity_method_cache: dict[tuple[type, str], bool] = {}
3168+
# When the caller supplies a cache (the worker does), reflected entity
3169+
# method signatures are shared with it and therefore survive beyond the
3170+
# lifetime of this executor.
3171+
self._entity_method_cache: _EntityMethodSignatureCache = (
3172+
entity_method_cache
3173+
if entity_method_cache is not None
3174+
else _EntityMethodSignatureCache()
3175+
)
31173176

31183177
def execute(
31193178
self,

0 commit comments

Comments
 (0)