Skip to content

Commit e969d30

Browse files
Bernd VerstCopilot
andcommitted
Preserve legacy asdict semantics for values outside the fast paths
The single-pass walker replaced dataclasses.asdict, but a lot of compatibility behavior lived inside asdict rather than in the conversion pass that followed it: container subclasses were rebuilt through their own constructors, mapping keys were recursed, namedtuples and defaultdict were special-cased, and every leaf was deep-copied. Walking values in place silently dropped all of that. Restrict the inline fast paths to exact built-in types, where walking in place is provably identical to what asdict produced, and hand every other value back to the real asdict via a throwaway box dataclass. Delegating rather than reimplementing matters because those internals have shifted between releases and this package supports 3.10 through 3.14. Also remove the tuple branch added earlier. Normalizing datetimes inside tuples is a real bug fix, but a user-visible one, so it does not belong in a performance change. Tuples now route to the compatibility path and come back exactly as they did before, quirk included. Bound the field-name cache with functools.lru_cache so dynamically created dataclass types cannot pin an unbounded number of entries. It measures identically to the unbounded dict (0.0204s vs 0.0206s per 200k lookups) while dropping the cache entirely is 9.4x slower, so the cache stays but is now capped. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 6322dc7f-81ee-4d42-be2d-cef84cf62d15
1 parent e2f8065 commit e969d30

2 files changed

Lines changed: 341 additions & 83 deletions

File tree

durabletask/history.py

Lines changed: 66 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,9 @@
33

44
from __future__ import annotations
55

6-
import copy
6+
import functools
77
from collections.abc import Callable
8-
from dataclasses import dataclass, fields
8+
from dataclasses import asdict, dataclass, fields
99
from datetime import datetime, timezone
1010
from typing import Any, cast
1111

@@ -332,22 +332,60 @@ def _message_to_dict(msg: Message) -> dict[str, Any]:
332332

333333
# Field names are looked up once per dataclass type. History export walks
334334
# many events of the same handful of types, so caching avoids repeatedly
335-
# rebuilding the tuple returned by ``dataclasses.fields``.
336-
_FIELD_NAMES: dict[type[Any], tuple[str, ...]] = {}
335+
# rebuilding the tuple returned by ``dataclasses.fields``. The cache is
336+
# bounded so that dynamically created dataclass types cannot pin an
337+
# unbounded number of entries (and the classes they reference) in memory.
338+
_FIELD_NAMES_CACHE_SIZE = 256
337339

338340
# Values of these exact types are already JSON-native and need no
339341
# conversion. Checking ``type(value)`` against a set is a single hash
340342
# lookup, which short-circuits the common case (most event fields are
341-
# strings, ints, or ``None``) before the ``isinstance`` chain below.
343+
# strings, ints, or ``None``) before the type checks below.
342344
_JSON_NATIVE_TYPES: frozenset[type[Any]] = frozenset({bool, float, int, str, type(None)})
343345

344346

347+
@functools.lru_cache(maxsize=_FIELD_NAMES_CACHE_SIZE)
345348
def _field_names(cls: type[Any]) -> tuple[str, ...]:
346-
names = _FIELD_NAMES.get(cls)
347-
if names is None:
348-
names = tuple(field.name for field in fields(cast(Any, cls)))
349-
_FIELD_NAMES[cls] = names
350-
return names
349+
return tuple(field.name for field in fields(cast(Any, cls)))
350+
351+
352+
@dataclass
353+
class _LegacyBox:
354+
"""Carrier used to re-enter ``dataclasses.asdict`` for one value."""
355+
356+
value: Any
357+
358+
359+
def _asdict_only(value: Any) -> Any:
360+
"""Apply ``dataclasses.asdict`` recursion to *value* and nothing else.
361+
362+
Boxing the value in a throwaway dataclass lets the interpreter's own
363+
``asdict`` implementation handle it, so container subclasses,
364+
namedtuples, ``defaultdict`` and the deep-copy of leaf values all behave
365+
exactly as they did before this module walked events itself. Delegating
366+
rather than reimplementing matters because those details have changed
367+
between Python releases and this package supports several of them.
368+
"""
369+
return asdict(_LegacyBox(value))['value']
370+
371+
372+
def _legacy_walk(value: Any) -> Any:
373+
"""The pre-optimization conversion pass, applied to an ``asdict`` result."""
374+
if isinstance(value, datetime):
375+
return value.isoformat()
376+
if isinstance(value, list):
377+
return [_legacy_walk(item) for item in cast(list[Any], value)]
378+
if isinstance(value, dict):
379+
return {
380+
key: _legacy_walk(item)
381+
for key, item in cast(dict[Any, Any], value).items()
382+
}
383+
return value
384+
385+
386+
def _legacy_compat(value: Any) -> Any:
387+
"""Reproduce the original ``asdict`` + walk pipeline for one value."""
388+
return _legacy_walk(_asdict_only(value))
351389

352390

353391
def _to_serializable(value: Any) -> Any:
@@ -356,10 +394,15 @@ def _to_serializable(value: Any) -> Any:
356394
This walks dataclass instances directly instead of going through
357395
``dataclasses.asdict``, which would deep-copy the whole event graph
358396
into a throwaway intermediate structure that then has to be walked a
359-
second time. Nested dataclasses become dicts in field order,
360-
datetimes become ISO 8601 strings, lists, tuples and dicts are
361-
rebuilt, and anything else is deep-copied so the result never shares
362-
mutable state with the event it came from.
397+
second time.
398+
399+
The type checks below are deliberately exact rather than
400+
``isinstance``. Only the built-in types are handled inline, because
401+
only for those is walking in place provably identical to what
402+
``asdict`` produced. Subclasses, tuples and every other value are
403+
routed to :func:`_legacy_compat`, which re-enters the real ``asdict``
404+
so their original semantics -- constructor round-trips, key
405+
recursion and deep-copied leaves -- are preserved exactly.
363406
"""
364407
value_type = cast('type[Any]', type(value))
365408
if value_type in _JSON_NATIVE_TYPES:
@@ -371,26 +414,20 @@ def _to_serializable(value: Any) -> Any:
371414
name: _to_serializable(getattr(value, name))
372415
for name in _field_names(value_type)
373416
}
374-
if isinstance(value, datetime):
417+
if value_type is datetime:
375418
return value.isoformat()
376-
if isinstance(value, list):
419+
if value_type is list:
377420
return [_to_serializable(item) for item in cast(list[Any], value)]
378-
if isinstance(value, tuple):
379-
items = [_to_serializable(item) for item in cast(tuple[Any, ...], value)]
380-
# Namedtuples take their fields as positional arguments rather than
381-
# a single iterable, so rebuilding them needs the unpacked form.
382-
if hasattr(value_type, '_fields'):
383-
return value_type(*items)
384-
return value_type(items)
385-
if isinstance(value, dict):
421+
if value_type is dict:
422+
# ``asdict`` recursed into keys but the conversion pass that followed
423+
# it did not, so keys get ``asdict`` semantics only. Native keys are
424+
# returned as-is because ``asdict`` leaves those untouched too.
386425
return {
387-
key: _to_serializable(item)
426+
(key if type(key) in _JSON_NATIVE_TYPES else _asdict_only(key)):
427+
_to_serializable(item)
388428
for key, item in cast(dict[Any, Any], value).items()
389429
}
390-
# ``asdict`` ended its recursion with ``copy.deepcopy``. Keeping that
391-
# behavior means callers can freely mutate the exported structure
392-
# without reaching back into the live event.
393-
return copy.deepcopy(value)
430+
return _legacy_compat(value)
394431

395432

396433
_EVENT_CONVERTERS: dict[str, Callable[[pb.HistoryEvent], HistoryEvent]] = {

0 commit comments

Comments
 (0)