[andystaples]

Summary

Focused follow-up to #310, inspected at its merged commit d2a3e7f together with the inline review discussion. Independent security and correctness reviewers also inspected the combined change from d2a3e7f^ through this PR.

The merged implementation already calls custom hooks once: it sorts canonical JSON documents, parses each document, and re-encodes the resulting values. This PR removes that intermediate parsing rather than claiming to fix the earlier 1,024-hook-call revision.

• Normalize only set subtrees once using the existing custom-object policy, then emit those same normalized values.
• Let list.sort(key=json.dumps) compute and cache one ordering representation per element for each multi-element set. Empty and singleton sets need no ordering representations.
• Keep JSON escaping, number formatting, and final emission in the standard library. The implementation is one small normalization helper, not a replacement JSON encoder.
• Detect cycles across custom hooks, containers, and nested sets while accepting shared, non-circular values.
• Fix two opt-in reconstruction gaps identified in the full-stack review: discovery recognizes sets of supported typed tuples, and bare collections.abc.Set annotations reconstruct arrays into concrete sets.
• Address dictionary-heavy normalization overhead: bypass recursive calls for scalar fields, track dictionary cycles only when descending into a child, and perform UTF-16 conversion only for actual high/low surrogate pairs—not ordinary Unicode names or lone surrogates.

Compatibility and determinism

Only reconstruction is opt-in. Set serialization changes also affect the default converter. #310's set serialization has not been released; that is the compatibility allowance for the intentional set wire-format changes below. The recorded-array replay regression does not establish byte-for-byte compatibility for previously serialized sets.

Ordinary payload encoding outside set subtrees and default-off set reconstruction behavior are unchanged. enable_generic_sets and its alias remain opt-in. An executor regression replays committed history whose set-annotated dataclass fields contain ordered arrays with duplicates, and verifies the original activity choice and output bytes.

Reconstruction corrections apply only when enabled. Both opt-in names are covered through deserialization/coercion, activity and orchestrator input discovery, activity-result discovery, and activity execution. Fixed-length and homogeneous tuples recurse through supported element types; tuples containing unhashable collection shapes remain excluded.

Lexical JSON ordering is retained, including {2, 10} -> [10, 2], non-finite floats, disjoint/nested frozensets, escaped strings, and dictionary insertion-order collisions. Native-container versus custom-hook precedence is preserved; hooks run once per occurrence, not once per object identity globally.

Inside set elements, dictionary names are sorted after conversion to normalized JSON strings. Thus {2: "two", 10: "ten"} emits {"10": "ten", "2": "two"}, just like those string names. Equivalent surrogate-pair/non-BMP names collapse with the original sorted traversal's last value winning, preserving the previous parser's normalization without parsing a document. Lone surrogates remain intact. This intentionally changes some previously serialized set bytes and potentially element ordering, even with the default converter.

Measurements at 943516d

Windows / CPython 3.14.10; merged source and follow-up loaded into separate modules in the same process. Times below are medians of nine batches, alternating implementation order; batches target roughly 100 ms, capped at 1,000 serializations. Peak traced allocations are from a separate single serialization, excluding prebuilt inputs and timing instrumentation. These representative synthetic measurements replace the earlier pre-review measurements at 4dad104; they are not universal performance guarantees.

Input	Merged ms	Follow-up ms	Speedup	Peak KiB, merged -> follow-up
10 integers	0.0355	0.0205	1.73x	3.4 -> 2.5
1,000 integers	3.7514	1.6396	2.29x	83.1 -> 61.2
1,000 strings	2.6916	0.8445	3.19x	117.6 -> 75.6
10 singleton-frozenset layers around a custom object	0.0722	0.0282	2.57x	17.3 -> 4.3
8 branching nested-frozenset layers	0.1220	0.0847	1.44x	16.4 -> 5.6
100 small custom objects	0.6343	0.7225	0.88x	38.0 -> 37.0
Custom singleton containing a 1 MiB string	8.4013	3.0941	2.72x	3586.6 -> 2562.0
4 custom objects, each with a 256 KiB string	8.1190	7.0915	1.14x	2435.4 -> 1411.9
4 custom objects, each with 10,000 integers	7.6963	6.6413	1.16x	1688.8 -> 622.8
10,000 Unicode-key dictionary fields	8.0532	5.9462	1.35x	1654.1 -> 1009.3
1,000 three-field ASCII dictionary records	1.7796	1.8606	0.96x	292.6 -> 231.2

Both implementations call the custom hook once, not 1,024 times, for the ten-layer singleton case. A no-set control uses the unchanged encoding path; timing variation there is not attributed to this PR.

Reviewer payloads: before and after the dictionary optimization

The reviewer correctly identified larger regressions than the original benchmark covered. Their Windows / CPython 3.13 measurements were 2.58x slower for Unicode names and 1.65x slower for ASCII records. The following local comparison uses CPython 3.14.10, not 3.13: seven batches of 30 serializations, alternating forward/reverse order across all three implementations. Each input is a singleton set of an identity-hashable hook object returning the indicated value. Outputs match byte-for-byte, and every implementation invokes the hook once.

Hook result	Merged d2a3e7f	Before fix 90d8703	After fix 943516d	After / merged
{ "\u2603" + str(i): i for i in range(10000) }	8.8218 ms	21.6778 ms	6.2592 ms	0.710x
[{"a": i, "b": i + 1, "c": i + 2} for i in range(1000)]	1.9748 ms	3.1389 ms	2.0472 ms	1.037x

The ASCII case is still slower, not fixed to a guaranteed speedup. Repeated local runs varied; after the final dispatch optimization the residual slowdown measured about 4–11%. The maintainer's performance thread remains open for confirmation on their environment.

Remaining tradeoffs

Multi-element sets still need temporary serialized ordering keys and a final JSON encoding pass. The improvement removes stringify/parse/re-encode round trips and skips keys entirely for singletons; it does not claim zero intermediate strings or a single traversal by the JSON encoder. Python-side normalization is not universally faster: small custom objects and dictionary records retain overhead, as explicitly shown above. A structural-key prototype was rejected because it duplicated subtle JSON comparison rules and substantially regressed large numeric lists.

Reproduce the expanded benchmark matrix

Run from the repository root with the development environment's Python. BASE identifies the actual merged implementation, not an earlier revision of #310. To reproduce this snapshot exactly, load the follow-up with git show 943516d:durabletask/serialization.py. For the three-way reviewer comparison, also load 90d8703:durabletask/serialization.py, use seven batches of 30 calls, and alternate (merged, previous, current) with (current, previous, merged).

import gc
import statistics
import subprocess
import sys
import timeit
import tracemalloc
import types
from pathlib import Path

BASE = "d2a3e7f"

def load(name, source):
    module = types.ModuleType(name)
    sys.modules[name] = module
    exec(compile(source, name, "exec"), module.__dict__)
    return module

old = load("merged_codec", subprocess.check_output(
    ["git", "show", f"{BASE}:durabletask/serialization.py"], text=True))
new = load("followup_codec", (Path("durabletask") / "serialization.py").read_text())

class Box:
    def __init__(self, value):
        self.value = value
    def to_json(self):
        return self.value

nested = Box({"z": 1, "a": 2})
for _ in range(10):
    nested = frozenset({nested})
branching = frozenset({Box({"z": 1, "a": 2}), Box({"b": [1, 2]})})
for _ in range(8):
    branching = frozenset({branching, Box("sibling")})

cases = {
    "integers_10": set(range(10)),
    "integers_1000": set(range(1000)),
    "strings_1000": {f"item-{i}" for i in range(1000)},
    "nested_singletons_10": nested,
    "nested_branching_8": branching,
    "custom_objects_100": {Box({"z": i, "a": [1, 2, 3]}) for i in range(100)},
    "large_singleton_1MiB": {Box({"payload": "x" * 1048576})},
    "large_strings_4x256KiB": {
        Box({"payload": str(i) + "x" * 262144}) for i in range(4)},
    "numeric_lists_4x10000": {
        Box(list(range(i, i + 10000))) for i in range(4)},
    "unicode_keys_10000": {
        Box({"\u2603" + str(i): i for i in range(10000)})},
    "ascii_records_1000": {
        Box([{"a": i, "b": i + 1, "c": i + 2} for i in range(1000)])},
    "no_sets_control": list(range(10000)),
}
for name, value in cases.items():
    assert old._to_json(value) == new._to_json(value)
    count = max(3, min(1000, int(0.1 / max(
        timeit.timeit(lambda: old._to_json(value), number=1), 0.000001))))
    samples = {old: [], new: []}
    for repetition in range(9):
        order = (old, new) if repetition % 2 == 0 else (new, old)
        for module in order:
            samples[module].append(timeit.timeit(
                lambda: module._to_json(value), number=count) / count * 1000)
    stats = []
    for module in (old, new):
        gc.collect()
        tracemalloc.start()
        module._to_json(value)
        peak = tracemalloc.get_traced_memory()[1] / 1024
        tracemalloc.stop()
        stats.append((statistics.median(samples[module]), peak))
    print(name, stats)

Review follow-up and validation

• Reproduced and fixed the agentic Unicode object-name collision finding; consolidated the test module imports. Replied to and resolved those three GitHub review threads in 4e83dee.
• Independent security review of fix(serialization): support set and frozenset generic containers in JsonDataConverter #310 plus this PR found no actionable vulnerabilities. It used source inspection and targeted probes, not live service integration.
• Independent correctness review found typed-tuple discovery and bare abstract-set reconstruction gaps from fix(serialization): support set and frozenset generic containers in JsonDataConverter #310. Both were fixed in 90d8703, and the same reviewer confirmed no remaining blockers for those fixes with 93 focused tests passing.
• Maintainer performance feedback is addressed by the dictionary optimizations in 943516d and the expanded measurements above. The thread remains open for their confirmation of the remaining tradeoff.
• 506 focused tests passed across core serialization/converters/type discovery/orchestration execution, Azure Managed worker/interceptor units, and Azure Functions serialization/converter units.
• Hash-seed subprocess coverage uses 0, 1, 2, 42, and 999, including numeric/string object-name normalization.
• Regression tests cover hooks through ten nested frozensets, no intermediate JSON parsing, no singleton ordering documents, one ordering key per element, canonical lexical ordering, Unicode names and lone surrogates, cycles, shared values, native-container precedence, encoding failures, committed-history replay, and opt-in discovery/execution compatibility. The reviewer payload shapes and dictionary cycles through dictionary/list/hook children are now included.
• Another 500 seeded randomized comparisons of nested JSON-compatible values matched the merged serializer byte-for-byte; intentional object-name normalization changes are covered separately.
• Strict Pyright for durabletask/serialization.py, separate core source/test flake8 checks, autopep8 diff, and git diff --check are clean.
• Scheduler/host-dependent E2E tests were not run locally. No dependency or package version changes.

[Copilot]

🟡 Changes recommended

Equivalent surrogate-pair and non-BMP object keys can produce duplicate JSON property names, bypassing last-value-wins normalization and changing payloads beyond the intended ordering change.

1 open issue

• Normalize equivalent Unicode object names before insertion · New

What changed in this PR

This follow-up to #310 optimizes deterministic set serialization in the core SDK by removing intermediate JSON parsing while preserving ordinary payload encoding and opt-in set reconstruction.

Changes:

• Normalize set subtrees once, skipping ordering keys for empty and singleton sets.
• Sort JSON object names after string conversion within set elements.
• Add regression coverage for ordering, hook calls, cycles, and committed-history replay.

File	Description
tests/​durabletask/​test_serialization.py	Tests normalization, ordering, hook behavior, cycles, and encoding compatibility.
tests/​durabletask/​test_data_converter.py	Verifies replay preserves recorded arrays under set annotations.
durabletask/​serialization.py	Replaces JSON parse round trips with set-subtree normalization.
CHANGELOG.md	Documents serialization improvements and object-name ordering changes.

_{Review effort: Balanced}

[Bernd Verst (berndverst)]

The implementation looks functionally sound within the documented allowance to change the unreleased set wire format. I did not find another actionable correctness defect, but the dictionary-heavy performance regressions below should be addressed before merging an optimization PR.

One compatibility qualification: only set reconstruction is opt-in. The intentional object-name normalization changes set serialization bytes and potentially element ordering even with the default converter. The recorded-array replay test is useful, but does not establish byte-for-byte compatibility for previously serialized sets; the unreleased status of this behavior is therefore important.

[Bernd Verst (berndverst)]

[P2] Avoid the substantial regression for dictionary-heavy set elements

The Python normalization walk adds significant overhead for dictionary-heavy payloads. In particular, this branch performs a UTF-16 encode/decode for every non-ASCII object name, including ordinary Unicode names that contain no surrogate pairs.

Paired benchmarks against the merged implementation (d2a3e7f) on Windows / CPython 3.13.13, loading both implementations into separate modules in the same process, reproduced these regressions. Each input is a singleton set containing an identity-hashable object whose to_json() returns the indicated value. Outputs matched byte-for-byte. These are medians of seven batches of 30 serializations, alternating implementation order:

Hook result	Merged	PR	Slowdown
{ "\u2603" + str(i): i for i in range(10000) } (the prefix is U+2603)	7.63 ms	19.71 ms	2.58x
[{"a": i, "b": i + 1, "c": i + 2} for i in range(1000)]	1.64 ms	2.71 ms	1.65x

These exceed the documented 9% small-object slowdown and also apply with enable_generic_sets=False, since that option gates reconstruction only.

Please add these shapes to the performance comparisons and optimize dictionary normalization before merging. Avoiding UTF-16 conversion when no surrogate composition is needed would address one source of overhead, but the ASCII nested-record case also needs attention.

[andystaples]

Reproduced both regressions against d2a3e7f and pushed a focused optimization in 943516d:

• Ordinary Unicode names and lone surrogates no longer undergo UTF-16 encode/decode; only actual high/low surrogate pairs do.
• Scalar dictionary fields bypass recursive normalization, and scalar-only dictionaries avoid cycle-tracking writes. Compound children still track their parent while traversing it, with new dictionary/list/hook cycle regressions.

I added both shapes to the PR's expanded performance comparisons and regression coverage. On this machine's CPython 3.14.10 (not your 3.13 environment), seven batches of 30 serializations with alternating forward/reverse order across merged / pre-fix / fixed source produced:

Hook result	Merged d2a3e7f	Pre-fix 90d8703	Fixed 943516d
10,000 U+2603-prefixed fields	8.8218 ms	21.6778 ms	6.2592 ms
1,000 ASCII three-field records	1.9748 ms	3.1389 ms	2.0472 ms

Outputs matched byte-for-byte, with one hook call each. Peak traced allocations for the two fixed cases were 1009.3 / 231.2 KiB versus the merged implementation's 1654.0 / 292.6 KiB. ASCII records remain slightly slower: the final implementation measured roughly 4–11% overhead across repeated local runs, so I'm not claiming a universal speedup or hiding that tradeoff. The expanded matrix also retains the small-custom-object regression.

I also clarified the compatibility section: only reconstruction is opt-in; serialization changes affect the default converter, and the recorded-array replay test does not prove compatibility for previously serialized sets. The unreleased status of #310 is the allowance for those wire changes.

Leaving this thread open for confirmation on your environment rather than treating my different-interpreter measurements as sufficient to close your concern.