Skip to content

gh-xxxxx: Add type-local type cache #130135

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

DinoV
Copy link
Contributor

@DinoV DinoV commented Feb 14, 2025

Adds a new L1 cache for types in the free-threaded build. The L1 cache is a small type-local cache which requires no locking to lookup the values.

The cache only adds objects and never replaces them, meaning that once we've confirmed the name is correct, we know the value. The cache also stores the version it's valid for, guaranteeing that we report the correct version for the lookup.

The cache is stored in the type object and is freed with qsbr when the type object is modified. Objects in the cache are eagerly marked as maybe weakref'd allowing the lookup to do a simple incref.

Unlike the normal method cache we do support probing on this cache. Values that are successfully stored in the l1 cache are not duplicated into the l2 cache reducing pressure that causes thrashing on the l2 cache.

We have some slightly different invariants about what we cache here. We won't cache lookups against types which have an odd tp_getattro. Modules and meta-classes are pretty representative of these types, and those lookups tend to be things which are just going to fill up the cache and we'll just continuously get misses on lookups.

We also stop caching if a type has exhausted its version allocation to prevent repeatedly re-allocating the local type cache on the same type.

This shows pretty good results on the benchmarks (being ~2% faster), but it might be better in real world cases. What is now the l2 cache can see a lot of trashing of values where values bounce in and out of the cache when collisions happen. In the free-threaded build that thrashing now involves taking the type lock to search the MRO which is going to be much more expensive than the thrashing in non-free threaded builds. So the real world impact when lots of methods are contending for cache space may be more dramatic.

This cache might be able to serve other purposes as well. One possibility is that we could use it to provide access to non-deferred objects in the specializing interpreter. We can cache with a "hint" location in the cache and as long as the type isn't modified we'd have safe access to the reference in the cache. I'm not sure if this is actually worth it as it seems like things which we frequently can't defer we can't specialize anyway.

https://github.com/facebookexperimental/free-threading-benchmarking/blob/main/results/bm-20250213-3.14.0a5%2B-68b535b-NOGIL/bm-20250213-vultr-x86_64-DinoV-local_type_cache-3.14.0a5%2B-68b535b-vs-base.md

@DinoV DinoV force-pushed the local_type_cache branch 3 times, most recently from 517abeb to f6f1dd5 Compare February 14, 2025 20:46
@DinoV DinoV changed the title Add type-local type cache gh-xxxxx: Add type-local type cache Feb 14, 2025
@DinoV DinoV force-pushed the local_type_cache branch 4 times, most recently from 106607c to 627b63e Compare February 14, 2025 22:10
@Fidget-Spinner
Copy link
Member

This is really impressive! I have a feeling this could eliminate the need for a polymorphic inline cache too in the interpreter.

@DinoV
Copy link
Contributor Author

DinoV commented Feb 15, 2025

This is really impressive! I have a feeling this could eliminate the need for a polymorphic inline cache too in the interpreter.

That's an interesting idea! The reason I started looking at this is largely because polymorphoc cases hit the cache pretty heavily. But we could use a hint index in the polymorphic cases that is shared across the class hierarchy as it only depends on the name. As long as the hierarchy has similar items (eg all functions, and not a mix of other descriptor types) it may be able to hit without caring about the class so much.

@DinoV DinoV force-pushed the local_type_cache branch 7 times, most recently from aa60561 to e71a0e9 Compare February 20, 2025 23:58
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.

2 participants