[3.10] bpo-46006: Revert "bpo-40521: Per-interpreter interned strings (GH-20085)" (GH-30422)

[vstinner]

This reverts commit ea25180.

Keep "assert(interned == NULL);" in _PyUnicode_Fini(), but only for
the main interpreter.

Keep _PyUnicode_ClearInterned() changes avoiding the creation of a
temporary Python list object.

(cherry picked from commit 35d6540)

https://bugs.python.org/issue46006

[vstinner]

@encukou and @pablogsal: the "Check if the ABI has changed" job failed with:

 underlying type 'struct _is' at pycore_interp.h:220:1 changed: type size changed from 908160 to 908096 (in bits) 

That's right. My PR changes PyInterpreterState which is excluded from the limited C API / stable ABI. How can I merge my PR (fix the CI job)?

[pablogsal]

Technically we cannot merge this PR then if that changes the ABI.

We need to discuss if the change is small enough to justify solving it this way.

@ambv@encukou

[pablogsal]

@vstinner could you email python Dev about this ABI change? I think this needs to be discussed broadly before we merge (or not) into 3.10

Alternatively, you can restore the pointer and leave it to be NULL.

[vstinner]

The last time I saw a bug report about an internal structure, the issue was closed as "not a bug": https://bugs.python.org/issue39599

Well, I don't want to bother about this specific revert. It seems like a few persons like to inspect PyInterpreterState structure at runtime in debuggers and profilers, and changing PyInterpreterState is just an annoyance. So I will just do what Pablo suggests, keep the member but don't use it.

[vstinner]

Tests / Check if the ABI has changed (pull_request) Successful in 2m

Good.

[vstinner]

I cannot merge this PR because of an "unresolved conversation", but I can no longer access the conversation, since it's gone after a git push --force. So I created a copy: PR #30433.

[kumaraditya303]

@vstinner As I clicked on conversations it seems it got resolved.

[vstinner]

@vstinner As I clicked on conversations it seems it got resolved.

You're a magician! You unblocked my PR.

[ericsnowcurrently]

How did something out of Include/internal end up in the stable ABI? That shouldn't have been possible.

[vstinner]

How did something out of Include/internal end up in the stable ABI? That shouldn't have been possible.

It's excluded from the limited C API. I'm not sure why @pablogsal and @encukou care about it.

As I wrote, I'm aware that a few persons rely on the exact PyInterpreterState structure, so if it's possible to avoid modifying it, I prefer to leave it as it is in a Python stable version (3.10.x). In Python 3.11, it's fine to change it since Python 3.11 is not released yet.

[pablogsal]

How did something out of Include/internal end up in the stable ABI? That shouldn't have been possible.

It's excluded from the limited C API. I'm not sure why @pablogsal and @encukou care about it.

As I wrote, I'm aware that a few persons rely on the exact PyInterpreterState structure, so if it's possible to avoid modifying it, I prefer to leave it as it is in a Python stable version (3.10.x). In Python 3.11, it's fine to change it since Python 3.11 is not released yet.

We care about it because although is not in the limited C-API, there are projects such as profilers and debuggers that rely on the size of the struct and changing this in a bugfix release is a pain.

[vstinner]

We care about it because although is not in the limited C-API, there are projects such as profilers and debuggers that rely on the size of the struct and changing this in a bugfix release is a pain.

Does it mean that the GitHub ABI CI doesn't only check the stable ABI, but the whole ABI?

[pablogsal]

Does it mean that the GitHub ABI CI doesn't only check the stable ABI, but the whole ABI?

You can see what it does:

cpython/Makefile.pre.in

Line 761 in bea3f42

abidiff $(srcdir)/Doc/data/python$(LDVERSION).abi "libpython$(LDVERSION).so" --drop-private-types --no-architecture --no-added-syms

It checks from all the symbols exposed from the shared object, which is a superset of the stable ABI.

[encukou]

It's excluded from the limited C API. I'm not sure why @pablogsal and @encukou care about it.

The stable ABI (and limited API) should be stable across all 3.x versions.
But maintenance branches should not break any ABI – not just the limited subset.

[ericsnowcurrently]

But maintenance branches should not break any ABI – not just the limited subset.

Right. I wasn't thinking about backports when I asked about the stable ABI. We definitely should not be breaking any ABI in a bugfix release. The CI check that caught the ABI change is definitely valuable.