Performance [P3]: Serialize history events in one pass

[berndverst]

Summary

HistoryEvent.to_dict() was implemented as _to_serializable(asdict(self)). dataclasses.asdict() recursively deep-copies the entire event graph into a throwaway intermediate structure, and _to_serializable() then walked that copy again to convert datetimes and rebuild containers. That is two full traversals plus a discarded allocation per event, multiplied across every event in a history export.

This replaces it with a single-pass recursive serializer that reads dataclass fields directly:

• _to_serializable() now handles dataclass instances itself (converting them to dicts in declaration order) instead of relying on asdict() to pre-flatten them.
• A fast path returns exactly-JSON-native values (str, int, float, bool, None) before any further checks — most event fields hit it.
• Field-name tuples are cached per dataclass type via functools.lru_cache, since history export walks many events of the same handful of types.

The dataclass check mirrors the private dataclasses._is_dataclass_instance predicate that asdict() used (hasattr(type(v), '__dataclass_fields__')), so dataclass types remain leaves and only instances recurse.

Preserving the asdict contract

Review correctly caught that the first version of this change did more than optimize. Most of the compatibility behavior of the old pipeline lived inside asdict, not in the _to_serializable pass that followed it, and walking values in place silently dropped it. asdict rebuilds list/tuple/dict subclasses through their own constructors, recurses into mapping keys, special-cases namedtuples and defaultdict, and copy.deepcopys every leaf.

The fix is to make the inline fast paths exact-type rather than isinstance, so they apply only where walking in place is provably identical to the old output, and to hand every other value back to the real asdict:

@dataclass
class _LegacyBox:
    value: Any

def _asdict_only(value):
    return asdict(_LegacyBox(value))['value']

Boxing a single value in a throwaway dataclass re-enters the interpreter's own asdict, so subclass constructor protocols, key recursion, defaultdict, namedtuples and leaf deep-copying all behave exactly as before. Delegating rather than reimplementing matters because those internals have shifted between releases and this package supports 3.10 through 3.14 — a hand-rolled copy risks divergence that only shows up on one CI matrix leg.

That is not hypothetical, and this PR hit it. Until 3.12, _asdict_inner rebuilt every dict subclass with type(obj)(<generator>), which defaultdict rejects:

TypeError: first argument must be callable or None

3.12 added a dedicated branch that forwards default_factory. So the legacy pipeline raises for a defaultdict on 3.10 and 3.11 and succeeds from 3.12 on. Delegation reproduces both for free — the parity test asserting the walker fails the same way legacy does passes on every version. Had the branches been hand-rolled from a 3.13 reading of _asdict_inner, the walker would have happily serialized a value that the code it replaced could not, on exactly two of the five matrix legs.

Concretely, versus the first revision of this PR:

input	first revision	now
datetime subclass overriding __deepcopy__	converted from the original	converted from the copy, as before
list subclass	constructor skipped	constructor re-runs, as before
dict subclass	constructor skipped, keys reused	constructor re-runs, keys recursed
mapping key with __deepcopy__ → str	left in place, json.dumps raises	deep-copied, serializes again
tuple subclass with iterator-only constructor	TypeError	rebuilt from a generator, as before
unrecognized mutable leaf	returned by reference	deep-copied

The tuple branch is removed entirely. Normalizing datetimes inside tuples is a real bug — the legacy output for those payloads cannot be JSON encoded — but it is user-visible behavior, so it does not belong in a performance change and will be raised separately. Tuples now route through the compatibility path and come back exactly as before, quirk included. TestTupleHandling::test_tuple_datetimes_remain_unencodable_like_legacy pins the current behavior so that follow-up has to be deliberate rather than accidental.

Cost

None. These paths are unreachable for SDK-produced events, so they were latent rather than live: the only dict[str, Any] field on any event is HistoryStateEvent.orchestration_state, populated exclusively by _message_to_dict → json_format.MessageToDict(...), which yields JSON-native values only. Re-benchmarked against the shipped module after the change, the walker is 6.0x faster than the original — the compatibility fallback never executes on the real corpus.

The field-name cache is now bounded (lru_cache(maxsize=256)) so that arbitrary user dataclass types nested under orchestration_state cannot grow it without limit. Measured over 200k lookups, best of 5: unbounded dict 0.0206s, lru_cache 0.0204s, no cache 0.1942s. The bound is free; removing the cache is not an option at 9.4x slower.

Output compatibility

Output is unchanged for every input that previously succeeded — same keys, same field ordering, same nesting, same datetime.isoformat() timestamps, same handling of None, enums, nested dataclasses, lists, and dicts — and inputs that previously raised still raise the same exception type. to_dict() still returns freshly built containers, and as with asdict, mutating any part of the result never touches the event.

No public API signature changed, nothing was renamed or removed, and no import path moved. _to_serializable keeps its name and signature.

Because this is now a pure internal performance change with no observable behavior difference, no changelog entry was added.

Verification

• Added tests/durabletask/test_history_serialization.py (157 tests), an equivalence suite that pins to_dict() against a verbatim copy of the old two-pass implementation (_to_serializable(asdict(event))) used as a test oracle. It covers all 28 HistoryEvent subclasses plus the base class (39 sample instances: fully populated and minimally populated variants), with nested dataclasses, nested dicts/lists, naive and aware datetimes, empty containers, and a deeply nested orchestration_state payload. Each event is compared with both == and repr(), so key ordering and value types are pinned, not just equality. A guard test asserts every exported HistoryEvent subclass is covered.
• Added TestLegacyCompatibility, a parametrized corpus of 22 exotic values — container subclasses, namedtuples, defaultdict, iterator-only tuple constructors, custom mapping keys, sets, bytearrays and custom objects — asserting both that output matches the legacy pipeline and that failures match it too. The oracle is asdict applied to the whole event, never to an individual value, so it cannot become circular with the implementation's own delegation. Verified on 3.11 and 3.13 locally, either side of the defaultdict behavior change, in addition to CI's five versions.
• RED → GREEN: these tests were confirmed failing against the previous revision before the fix landed — 30 failures, covering every point raised in review. Two fixtures had to be strengthened first: an idempotent subclass constructor and a datetime subclass overriding only isoformat both pass against the broken code, so they were replaced with a non-idempotent constructor and a __deepcopy__ override respectively.
• Mutation-checked the equivalence assertions to confirm they have teeth: replacing the serializer with the identity function, and reversing field order, are each detected on all 39 sample events.
• Core unit tests: 903 passed, 7 skipped (pytest tests/durabletask -q --ignore-glob="*_e2e.py").
• Provider unit tests: 63 passed (test_sandboxes_extension.py, test_durabletask_grpc_interceptor.py).
• history_export — the downstream consumer of to_dict() via event_to_dict() — 76 passed, 1 skipped.
• flake8 . clean from the repository root, a superset of every per-directory target CI lints.
• pyright strict at the configured pythonVersion of 3.10 reports 0 errors on both changed files.
• Benchmarked over the 39-event corpus (2000 iterations, best of 7), timing the real to_dict() rather than a transcription of it, so the number cannot drift from the code under review: original two-pass form 1.730s, single-pass walker 0.287s — 6.02x faster. Output is asserted byte-identical event by event before any timing runs.

Fixes #191

[Copilot]

Pull request overview

This PR optimizes history event serialization in the core Durable Task Python SDK by removing the two-pass asdict() + walker approach and replacing it with a single-pass serializer that walks dataclass fields directly, aiming to reduce allocations and CPU time when exporting large histories.

Changes:

• Reworked HistoryEvent.to_dict() and _to_serializable() to serialize dataclass instances directly (single pass) and cache per-type field name tuples.
• Added a fast-path for JSON-native scalar types to reduce overhead in common cases.
• Added a comprehensive equivalence test suite that pins the new output to the legacy asdict()-based behavior across all exported HistoryEvent types.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
durabletask/history.py	Replaces asdict()-based serialization with a cached, single-pass dataclass-aware serializer for history events.
tests/durabletask/test_history_serialization.py	Adds an oracle-based equivalence and isolation test suite to ensure to_dict() remains behavior-preserving.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

durabletask/history.py:389

• In the dict branch, values are rebuilt but keys are passed through unchanged. This breaks the stated isolation guarantee (“result never shares mutable state with the event”) for non-primitive / user-supplied keys, since keys will still alias the original objects (the legacy asdict(...); _to_serializable(...) path deep-copied dict keys as part of asdict). Deep-copying keys here keeps behavior closer to the legacy implementation without affecting the SDK’s normal str keys.

    if isinstance(value, dict):
        return {
            key: _to_serializable(item)
            for key, item in cast(dict[Any, Any], value).items()
        }

[andystaples]

Requesting changes because this PR is described as a performance-only optimization, but the new walker changes serializer behavior for customer-supplied values.

Please keep this PR scoped to serializing history events in one pass while preserving the legacy asdict() + _to_serializable() contract for every input that previously succeeded. Exact built-in fast paths are fine, but subclasses, custom containers, and mapping keys need compatibility handling.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

durabletask/history.py:389

• In the dict branch, only values are rebuilt/serialized; keys are passed through by reference. This breaks the stated “never shares mutable state” guarantee for hashable-but-mutable key objects, and diverges from dataclasses.asdict(), which deep-copies dict keys during recursion. Consider deep-copying keys when rebuilding the dict.

    if isinstance(value, dict):
        return {
            key: _to_serializable(item)
            for key, item in cast(dict[Any, Any], value).items()
        }

durabletask/history.py:337

• _FIELD_NAMES caches dataclass field-name tuples in a module-level dict keyed by class objects. Because this holds strong references to classes, it can keep dynamically-created dataclass types alive and grow without bound in long-running processes that serialize many distinct dataclass types (e.g., user-provided payloads). Consider using a weakref.WeakKeyDictionary (or a bounded cache) to avoid retaining classes indefinitely.

# Field names are looked up once per dataclass type. History export walks
# many events of the same handful of types, so caching avoids repeatedly
# rebuilding the tuple returned by ``dataclasses.fields``.
_FIELD_NAMES: dict[type[Any], tuple[str, ...]] = {}

[berndverst]

Thanks for the review — the framing was the useful part, and I agree with it. Most of the compatibility behavior of the old pipeline lived inside asdict rather than in the _to_serializable pass that followed it, and walking values in place quietly dropped it. That is a behavior change riding along with a performance change, which is not what this PR is for.

Rather than reimplement _asdict_inner's branches, the fast paths are now exact-type checks and everything else is handed back to the real asdict through a throwaway box dataclass. That inherits whatever the running interpreter does, which matters given CI spans 3.10 through 3.14 and those internals have moved between releases.

The tuple branch is removed outright rather than repaired, per your point that it belongs in a dedicated bug-fix PR.

Two things worth flagging because they changed how I tested this:

• My first regression test for the datetime subclass case passed against the broken code. Overriding only isoformat is not enough, since calling it on the original and on a deep copy agree. It only goes red with a __deepcopy__ override, which is the case you actually named.
• Same trap for the container subclasses: a constructor that only dedupes or uppercases is idempotent, so skipping it is invisible. The fixtures now append a marker each time they run, making the skipped constructor directly observable.

Both fixtures were strengthened before the fix landed. The new tests fail 30 times against the previous revision and pass now.

TestLegacyCompatibility is the durable artifact you asked for: a parametrized corpus of exotic inputs asserting the walker matches _to_serializable(asdict(...)), including matching failures, so the contract is executable and the next optimization cannot silently break it. The oracle applies asdict to the whole event rather than to individual values, so it never becomes circular with the implementation's own delegation.

Re-benchmarked as requested: still 6.1x, in fact marginally faster than before, since removing the tuple branch removed a check from the hot path. The compatibility work costs nothing on real payloads because those paths are unreachable — orchestration_state is fed exclusively by MessageToDict, which emits JSON-native types only.

CHANGELOG left alone: with the tuple branch gone there is no user-visible behavior change left to document.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

[berndverst]

Already reconciled — in the direction of your first option, matching the preserved-quirk behavior.

That text was from an earlier revision. The tuple branch was removed after review feedback that normalizing datetimes inside tuples is user-visible behavior and does not belong in a performance-only change, and the description was rewritten at the same time. It currently reads:

The tuple branch is removed entirely. Normalizing datetimes inside tuples is a real bug — the legacy output for those payloads cannot be JSON encoded — but it is user-visible behavior, so it does not belong in a performance change and will be raised separately. Tuples now route through the compatibility path and come back exactly as before, quirk included.

So description and implementation agree: tuples are unchanged from main. These tests are the intended guard rather than a contradiction — test_tuple_datetimes_remain_unencodable_like_legacy pins the quirk so the follow-up fix has to be a deliberate act rather than an accidental side effect of some later refactor.

[berndverst]

Agreed, and fixed in 15c29eb.

The wording predates the compatibility rework and was accurate when written — at that point the walker converted everything it touched. Once tuples and other exotic shapes started routing back through _legacy_compat to preserve their original behavior, "JSON-safe" became a promise the function no longer keeps. A tuple holding a datetime is the clearest counterexample: it comes back with the datetime intact and json.dumps rejects it, exactly as before this PR.

The docstring now says which values are JSON-native and which are deliberately left alone:

Values the SDK itself produces convert to JSON-native types. Arbitrary values can also arrive through dict[str, Any] fields, and those keep whatever the original pipeline did with them -- which for a few shapes, such as a tuple holding a datetime, is not JSON-encodable. That is preserved on purpose rather than fixed here; see _legacy_compat.

Worth being explicit that this is a docstring correction, not a behavior change: the non-encodable output for those shapes is what main produces today, and preserving it is the point of the compatibility path.

[berndverst]

Follow-up: the 3.10 and 3.11 legs went red on the previous push, and the cause is worth recording here because it is direct evidence for the design choice above.

dataclasses.asdict rebuilt every dict subclass with type(obj)(<generator>) until 3.12, and defaultdict rejects that:

TypeError: first argument must be callable or None

3.12 added a dedicated branch forwarding default_factory. So the legacy pipeline raises for a defaultdict on 3.10 and 3.11 and succeeds from 3.12 on.

The implementation was not at fault and is unchanged. Because it delegates to the running interpreter's asdict, it reproduced both behaviors exactly — test_exotic_payload_raises_the_same_way_as_legacy[defaultdict] passed on 3.10, which is the assertion that actually matters. What failed was my strict-equality test, which assumed the legacy pipeline always yields a value to compare against. It now skips when legacy cannot serialize the payload at all and leaves that case to the parity test.

This is the failure mode that argued against hand-rolling _asdict_inner's branches in the first place. A version-agnostic reimplementation written against a 3.13 reading of _asdict_inner would have serialized a defaultdict successfully on 3.10 and 3.11 — where the code it replaced raises — and that divergence would have been invisible on three of the five matrix legs. Added test_defaultdict_follows_the_running_interpreter so the next person to look at this finds the reason attached to the behavior.

Verified locally on 3.11 and 3.13, either side of the change, in addition to CI: 156 passed / 1 skipped on 3.11, 157 passed on 3.13. Head is e31bb48, test-only.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

[berndverst]

Benchmark correction, since I quoted a specific figure above.

My earlier number came from a harness that re-implemented both walkers side by side. That is fine for isolating one branch, but it measures a transcription rather than the code under review, and the transcription had drifted — it still carried the tuple branch this PR no longer has.

Re-measured by importing durabletask.history and timing the real HistoryEvent.to_dict(), so the figure cannot diverge from what ships:

corpus: 39 history events
legacy and head agree byte-for-byte on the real corpus

 legacy: best 1.730s  median 1.745s  (2000 iterations x 39 events)
   head: best 0.287s  median 0.290s  (2000 iterations x 39 events)

legacy -> head : 6.02x faster

6.02x, not the 6.1x I quoted. The compatibility fallback costs nothing measurable because it never executes on this corpus — every value reaches an exact-type fast path, which is the expected result given orchestration_state comes only from MessageToDict. The harness asserts byte-identical output event by event before it times anything, so a regression cannot be reported as a speedup.

PR description updated to the measured figures.

[andystaples]

The previous findings are addressed and the PR is substantially closer to a performance-only change. One compatibility gap remains: applying the legacy fallback independently to child values does not preserve the old pipeline's ordering (asdict(event) completes before _to_serializable begins).

This can turn a previously successful customer value into an exception, so I think it still needs to be resolved before approval. Please use the complete legacy pipeline for an event/container whenever its nested graph is not wholly eligible for the exact-type fast paths, and add coverage for interactions among multiple custom values rather than only isolated leaves.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.