gh-132380: Use unicode hash/compare for the mcache.

[nascheme]

This allows the type lookup cache to work with non-interned strings.

pyperformance results

• Issue: Lock contention inside _PyType_LookupRef #132380

[picnixz]

(just adding the skip news label so that you don't get pinged by the bot everytime you push, saying "failed checks")

[ngoldbaum]

I cherry-picked this PR onto the 3.14 branch and built CPython and LibCST. I had to combine Instagram/LibCST#1324 and Instagram/LibCST#1295 and also back out the Python-level caching I added in Instagram/LibCST#1295, since that's unnecessary with this PR.

I see substantially improved multithreaded scaling, although there's still some contention. Looking at the profiles, it seems like the contention is coming from GC pauses?

Here's the profile I record on 3.14b1 and the 3.14 branch with this PR applied, respectively:

https://share.firefox.dev/43bHvhH

https://share.firefox.dev/3GM0Xdu

Here's the profile using multiprocessing:

https://share.firefox.dev/42QR32I

[ngoldbaum]

(this is on a mac so I can't easily get python-level profiles and line numbers in LibCST's Python code)

[nascheme]

Thanks for the testing. I tested LibCST on Linux and also see a performance improvement. Running the following command in the numpy source folder:

python3 -m libcst.tool codemod --no-format strip_strings_from_types.StripStringsCommand numpy/_core

I get the elapsed run times:

base 3.13, getattr(): 30.02 sec
base 3.13, type_lookup(): 20.94 sec
3.13 + this PR, getattr(): 18.48 sec

[nascheme]

@colesbury This uses unicode string hash/compare instead of using the string pointer value. Unlike your suggestion, this doesn't use a separate lookup loop for the non-interned case, it just always uses the string hash/compare. pyperformance results show a small slowdown (0.4 %?), I was expecting worse since the non-interned case is so uncommon.

I can try a separate loop if you think that's worth pursing. The advantage of this approach is that it's fairly simple code-wise and I think would be a candidate to backport to 3.13 and 3.14. Perhaps for 3.15 we should try a per-thread cache.

[colesbury]

I don't think we should do it this way. I don't think it's worth suffering even a small performance penalty for a rare case (non-interned lookup keys), when we can support that without any performance hit.

[colesbury]

I don't think this caches non-interned strings. It seems to me that it allows non-interned strings as the lookup key, but the cache still only contains interned strings.

[nascheme]

Closing this since I agree with Sam that the performance hit is too much to pay for improving such a rare case.