Use PyMarshal_WriteToString for logic of _PyCode_ConstantKey

[mdboom]

Feature or enhancement

Proposal:

As @brandtbucher mentioned elsewhere, _PyCode_ConstantKey is getting to be quite large, subtle and error-prone. The unique key that PyMarshal_WriteToString creates would serve the same purpose and would replace the logic with something core and well-tested.

In addition to correctness, it should also be tested for any performance impact. It's possible it's moot since _PyCode_ConstantKey already allocates a bunch of objects, but we should confirm.

intern_constants also does mostly the same thing in a free-threaded build and maybe could take a similar approach to also remove a bunch of code that is hard to keep in-sync with everything else.

Has this already been discussed elsewhere?

No response given

Links to previous discussion of this feature:

No response

[mdboom]

Having experimented with this, I'm starting to believe this won't work. While using PyMarshal_WriteObjectToString does do the right thing for determining which constants should be deduplicated, it doesn't/can't actually do the recursive deduplication that the current code does.

For example, this test in test_compile.py::test_merge_constants:

        f1, f2 = lambda: "not a name", lambda: ("not a name",)
        f3 = lambda x: x in {("not a name",)}
        self.assertIs(f1.__code__.co_consts[1],
                      f2.__code__.co_consts[1][0])
        self.assertIs(next(iter(f3.__code__.co_consts[1])),
                      f2.__code__.co_consts[1])

You can't get the string in ("not a name",) to be same as "not a name" without recursing through the tuple. By the time you've done that, you end up writing something with similar complexity to what we already have and then just unnecessarily (at that point) doing the marshal serialization.

So I'm inclined to close this, but as @brandtbucher is much more versed in these things maybe he has other ideas that I am missing.

[brandtbucher]

Yeah, bummer. Oh well, at least the current code isn't broken or anything. Thanks for investigating!