Protocol conformance cache for generic types #83989
Conversation
DmT021
requested review from
rjmccall,
mikeash,
al45tair and
compnerd
as code owners
|
@swift-ci please test |
|
@swift-ci please test macOS |
|
After poking around a bit, the issue is that this change doesn't correctly handle the case when In that case, if the table is a relative table, this will read it incorrectly and crash. I'm working out the best way to test and fix this case. I'll give an update when I have it. |
|
That was fun. Testing the The fix is to replace That will handle both cases correctly. As far as testing goes, I built the freestanding stdlib by using The resulting binary is a regular macOS program, but with a statically linked "freestanding" libswiftCore that uses relative witness tables. |
This change adds a new type of cache (cache by type descriptor) to the protocol conformance lookup system. This optimization is beneficial for generic types, where the same conformance can be reused across different instantiations of the generic type. Key changes: - Add a `GetOrInsertManyScope` class to `ConcurrentReadableHashMap` for performing multiple insertions under a single lock - Add type descriptor-based caching for protocol conformances - Add environment variables for controlling and debugging the conformance cache - Add tests to verify the behavior of the conformance cache - Fix for swiftlang#82889 The implementation is controlled by the `SWIFT_DEBUG_ENABLE_CACHE_PROTOCOL_CONFORMANCES_BY_TYPE_DESCRIPTOR` environment variable, which is enabled by default. This reapplies swiftlang#82818 after it's been reverted in swiftlang#83770.
DmT021
force-pushed
the
wp/conformance-cache-by-descriptor-2
branch
from
6ccc69e to
091b005
Compare
|
@swift-ci Please test |
|
@swift-ci Please smoke test macOS |
|
@swift-ci Please test |
|
@mikeash Can we merge this again? Or you want me to add a test for when we have |
|
Honestly I'm not sure myself. Manual testing would be OK for now. Do you want to try that? Otherwise I'm happy to repeat my manual testing above with your final revision here. |
|
I'd try, yes, but I'm not sure how to invoke Never mind, I think I figured it out. |
|
@mikeash I was able to reproduce the crash on the previous commit before the fix, and verified that it no longer occurs after the fix. It looks like the |
|
Oh, it's probably the mismatch for the objc interop support between the compiler and the runtime built by the |
|
Oh yes, that would do it. The class metadata layout is different with ObjC interop on or off. Sounds like this is all good to go, then. |
|
Yeah. I checked again on |
|
@mikeash Can you merge please? |
|
Off we go! Thanks for working through the second round. Hopefully there is no third. 😄 |
This change adds a new type of cache (cache by type descriptor) to the protocol conformance lookup system. This optimization is beneficial for generic types, where the same conformance can be reused across different instantiations of the generic type.
Key changes:
GetOrInsertManyScopeclass toConcurrentReadableHashMapfor performing multiple insertions under a single lockThe implementation is controlled by the
SWIFT_DEBUG_ENABLE_CACHE_PROTOCOL_CONFORMANCES_BY_TYPE_DESCRIPTORenvironment variable, which is enabled by default.This reapplies #82818 after it's been reverted in #83770.