Skip to content

gh-155901: Fix exponential time when marshalling nested sets - #157128

Open
iamsharduld wants to merge 1 commit into
python:mainfrom
iamsharduld:gh-155901-marshal-nested-sets
Open

gh-155901: Fix exponential time when marshalling nested sets#157128
iamsharduld wants to merge 1 commit into
python:mainfrom
iamsharduld:gh-155901-marshal-nested-sets

Conversation

@iamsharduld

@iamsharduld iamsharduld commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

marshal.dumps() sorts the elements of a set by their marshalled form, so that the output does not depend on hash randomization (bpo-37596). The sort key of each element was computed with a nested _PyMarshal_WriteObjectToString() call. For an element that is itself a set, that call sorts the nested set again, and then the element is marshalled a second time to write it out. So the time doubled with every level of nesting: depth 20 takes 0.5 s on a debug build here, and each further level doubles it.

Fix

The order of each set is now computed once per top-level call and cached in the writer state (WFILE.set_orders, a pointer-keyed _Py_hashtable_t that maps a set to a tuple of its elements in marshalling order). The nested writer that computes the sort keys (w_set_key()) shares that cache and the nesting depth with its parent, and still has its own reference table, so a key stays independent of where the element appears in the stream.

The cache holds borrowed references to the sets, which are kept alive by the object being marshalled. An owned reference would change the result of the _PyObject_IsUniquelyReferenced() check in w_ref() for a set that is referenced once, and add FLAG_REF to the output.

For a chain of nested sets the time is now quadratic in the depth (the sort key of a nested set is still its full marshalled form), and the depth is bounded by MAX_MARSHAL_STACK_DEPTH. Depth 1000 takes 47 ms on the debug build; the reproducer from the issue is instant at any depth.

Output is unchanged

I compared marshal.dumps() output between the unfixed and fixed interpreters, with the same PYTHONHASHSEED, for format versions 0 to 6 over 139 objects: plain, nested and shared sets, interned and non-interned strings, a code object with set constants, and the code objects of 120 stdlib modules. All 973 outputs are byte-for-byte identical.

Other visible effects

  • Nesting through set elements now counts towards the depth limit while computing sort keys too, so a very deep chain of sets raises ValueError: object too deeply nested to marshal promptly instead of never finishing. The write path already enforced the limit.
  • The error of the nested writer is propagated instead of being folded into "unmarshallable object", so a disallowed code object inside a set now reports "marshalling code objects is disallowed".
  • The marshal.dumps audit event is raised once per call. It was also raised for every set element, because the sort keys went through _PyMarshal_WriteObjectToString().

Tests

  • test_nested_sets: round-trips a chain of 100 nested frozensets, and a shallow structure with sets shared between elements, for every format version. It hangs before the fix.
  • test_recursion_limit: extended with a chain of MAX_MARSHAL_STACK_DEPTH - 1 frozensets that round-trips, plus one more level that raises ValueError.
  • test_deterministic_sets: added a nested-set sample, to check that the cached order does not depend on the hash seed.

Ran test_marshal (also with -R 3:3), test_audit, test_compileall, test_py_compile, test_zipimport, test_importlib, test_code, the full suite with -j0, and make patchcheck on a macOS debug build.

marshal.dumps() sorts the elements of a set by their marshalled form,
computed by marshalling each element on its own. When an element is
itself a set, that repeats the sort of the nested set, so the time
doubled with every level of nesting.

Cache the order of each set for the duration of the top-level call and
share the cache, and the nesting depth, with the nested writer used for
the sort keys. The output is unchanged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant