gh-140328: Use interned versions of string constants if they're already present

[albertedwardson]

closes#140328

intern_constants() previously only interned string constants when should_intern_string() returned true, leaving equal strings duplicated even if an interned instance already existed. This change first checks the global and per-interpreter interned string tables and reuses an existing interned string when available. This applies only to string constants that come from the codeobjects; dynamically created strings are unaffected and are still created as new objects.

This makes string interning behavior more consistent: once a string is interned, subsequent equal constants can reuse the canonical object. It may slightly reduce memory usage and improves interaction with sys.intern().

The change adds a small lookup cost when loading code objects, which may affect eval(), exec(), modules loading and similar operations.

I haven't run pyperformance yet, but I plan to provide a benchmark run once I have access to an appropriate runner. Given the limited scope of the change, I expect both the performance impact and memory savings to be relatively small but measurable.

• Issue: Use interned versions of string constants if they're already present #140328

[albertedwardson]

will investigate why ft build fails

[sergey-miryanov]

Same here.

[albertedwardson]

it adds extra indentation (that I'd like to avoid) to the next code block like so:

 } else{#endifif (should_intern_string(v)){PyObject*w=v; _PyUnicode_InternMortal(interp, &v); if (w!=v){PyTuple_SET_ITEM(tuple, i, v); set_modified(modified)} } } }

[sergey-miryanov]

Ok, let's wait what say others for this case.

[colesbury]

I'm not sure this PR makes sense. This is a bunch of extra complexity and I don't see the point of it. I think there are two options that make sense to me:

• Leave things the way they are. Things are working - I'm not sure there's a need to change things unless there's an actual benefit.
• Intern all strings in code objects. It's simpler and I don't think there's a reason to limit interned strings to [a-zA-Z0-9_] anymore.

[albertedwardson]

extra complexity and I don’t see the point of it.

IMO this is mainly about correctness and consistency of interning, not expanding its scope. Once a string is interned, keeping duplicate equal constants around feels unnecessary.

I don't think there’s a reason to limit interned strings to [a-zA-Z0-9_] anymore.

Could you please clarify the original reason for limiting interning to “identifier-like” strings? I believe there were some practical concerns

[colesbury]

Could you please clarify the original reason for limiting interning to “identifier-like” strings? I believe there were some practical concerns

It's not stated explicitly anywhere and was introduced almost 30 years ago:

7b89b6a

I suspect it's because:

1. The perf benefit from interning primarily helped names because there was a faster lookup in dicts (used for object attributes and globals).
2. Interned strings were never freed

In the GIL-enabled build, interned strings are freed when they are no longer referenced so (2) is no longer true. The free threaded build doesn't use this logic, so it's not really relevant here either.