Protocol conformance cache for generic types

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 https://github.com/swiftlang/swift/issues/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 https://github.com/swiftlang/swift/pull/82818 after it's been reverted in https://github.com/swiftlang/swift/pull/83770.
2025-12-14 20:36:38 +01:00 · 2025-08-28 23:47:41 +02:00
parent 44ba366759
commit 091b005a60
8 changed files with 550 additions and 114 deletions
--- a/include/swift/Runtime/Concurrent.h
+++ b/include/swift/Runtime/Concurrent.h
@@ -593,6 +593,51 @@ private:
    }
  }

+  // Common implementation for `getOrInsert` and `GetOrInsertManyScope`
+  template <class KeyTy, typename Call>
+  void getOrInsertExternallyLocked(KeyTy key, const Call &call) {
+    auto indices = IndexStorage{Indices.load(std::memory_order_relaxed)};
+    auto indicesCapacityLog2 = indices.getCapacityLog2();
+    auto elementCount = ElementCount.load(std::memory_order_relaxed);
+    auto *elements = Elements.load(std::memory_order_relaxed);
+    auto *elementsPtr = elements ? elements->data() : nullptr;
+
+    
+    auto found = this->find(key, indices, elementCount, elementsPtr);
+    if (found.first) {
+      call(found.first, false);
+      return;
+    }
+
+    auto indicesCapacity = 1UL << indicesCapacityLog2;
+
+    // The number of slots in use is elementCount + 1, since the capacity also
+    // takes a slot.
+    auto emptyCount = indicesCapacity - (elementCount + 1);
+    auto proportion = indicesCapacity / emptyCount;
+    if (proportion >= ResizeProportion) {
+      indices = resize(indices, indicesCapacityLog2, elementsPtr);
+      found = find(key, indices, elementCount, elementsPtr);
+      assert(!found.first && "Shouldn't suddenly find the key after rehashing");
+    }
+
+    if (!elements || elementCount >= elements->Capacity) {
+      elements = resize(elements, elementCount);
+    }
+    auto *element = &elements->data()[elementCount];
+
+    // Order matters: fill out the element, then update the count,
+    // then update the index.
+    bool keep = call(element, true);
+    if (keep) {
+      assert(hash_value(key) == hash_value(*element) &&
+            "Element must have the same hash code as its key.");
+      ElementCount.store(elementCount + 1, std::memory_order_release);
+      indices.storeIndexAt(&Indices, elementCount + 1, found.second,
+                          std::memory_order_release);
+    }
+  }
+
 public:
  // Implicitly trivial constructor/destructor.
  ConcurrentReadableHashMap() = default;
@@ -684,6 +729,48 @@ public:
    return Snapshot(this, indices, elementsPtr, elementCount);
  }

+  /// A wrapper that allows performing several `getOrInsert` operations under
+  /// the same lock.
+  class GetOrInsertManyScope {
+    GetOrInsertManyScope() = delete;
+    GetOrInsertManyScope(const GetOrInsertManyScope &) = delete;
+    GetOrInsertManyScope &operator=(const GetOrInsertManyScope &) = delete;
+    GetOrInsertManyScope(GetOrInsertManyScope &&) = delete;
+    GetOrInsertManyScope &operator=(GetOrInsertManyScope &&) = delete;
+
+    ConcurrentReadableHashMap &Map;
+
+  public:
+    GetOrInsertManyScope(ConcurrentReadableHashMap &map) : Map(map) {
+      Map.WriterLock.lock();
+    }
+
+    ~GetOrInsertManyScope() {
+      Map.deallocateFreeListIfSafe();
+      Map.WriterLock.unlock();
+    }
+
+    /// Get an element by key, or insert a new element for that key if one is
+    /// not already present. Invoke `call` with the pointer to the element.
+    ///
+    /// `call` is passed the following parameters:
+    ///   - `element`: the pointer to the element corresponding to `key`
+    ///   - `created`: true if the element is newly created, false if it already
+    ///                exists
+    /// `call` returns a `bool`. When `created` is `true`, the return values
+    /// mean:
+    ///   - `true` the new entry is to be kept
+    ///   - `false` indicates that the new entry is discarded
+    /// If the new entry is kept, then the new element MUST be initialized, and
+    /// have a hash value that matches the hash value of `key`.
+    ///
+    /// The return value is ignored when `created` is `false`.
+    template <class KeyTy, typename Call>
+    void getOrInsert(KeyTy key, const Call &call) {
+      Map.getOrInsertExternallyLocked(key, call);
+    }
+  };
+
  /// Get an element by key, or insert a new element for that key if one is not
  /// already present. Invoke `call` with the pointer to the element. BEWARE:
  /// `call` is invoked with the internal writer lock held, keep work to a
@@ -703,48 +790,7 @@ public:
  template <class KeyTy, typename Call>
  void getOrInsert(KeyTy key, const Call &call) {
    typename MutexTy::ScopedLock guard(WriterLock);
-
-    auto indices = IndexStorage{Indices.load(std::memory_order_relaxed)};
-    auto indicesCapacityLog2 = indices.getCapacityLog2();
-    auto elementCount = ElementCount.load(std::memory_order_relaxed);
-    auto *elements = Elements.load(std::memory_order_relaxed);
-    auto *elementsPtr = elements ? elements->data() : nullptr;
-
-    auto found = this->find(key, indices, elementCount, elementsPtr);
-    if (found.first) {
-      call(found.first, false);
-      deallocateFreeListIfSafe();
-      return;
-    }
-
-    auto indicesCapacity = 1UL << indicesCapacityLog2;
-
-    // The number of slots in use is elementCount + 1, since the capacity also
-    // takes a slot.
-    auto emptyCount = indicesCapacity - (elementCount + 1);
-    auto proportion = indicesCapacity / emptyCount;
-    if (proportion >= ResizeProportion) {
-      indices = resize(indices, indicesCapacityLog2, elementsPtr);
-      found = find(key, indices, elementCount, elementsPtr);
-      assert(!found.first && "Shouldn't suddenly find the key after rehashing");
-    }
-
-    if (!elements || elementCount >= elements->Capacity) {
-      elements = resize(elements, elementCount);
-    }
-    auto *element = &elements->data()[elementCount];
-
-    // Order matters: fill out the element, then update the count,
-    // then update the index.
-    bool keep = call(element, true);
-    if (keep) {
-      assert(hash_value(key) == hash_value(*element) &&
-             "Element must have the same hash code as its key.");
-      ElementCount.store(elementCount + 1, std::memory_order_release);
-      indices.storeIndexAt(&Indices, elementCount + 1, found.second,
-                           std::memory_order_release);
-    }
-
+    getOrInsertExternallyLocked(key, call);
    deallocateFreeListIfSafe();
  }

--- a/stdlib/public/runtime/EnvironmentVariables.def
+++ b/stdlib/public/runtime/EnvironmentVariables.def
@@ -62,6 +62,11 @@ VARIABLE(SWIFT_DEBUG_ENABLE_SHARED_CACHE_PROTOCOL_CONFORMANCES, bool, true,

 #endif

+VARIABLE(SWIFT_DEBUG_ENABLE_CACHE_PROTOCOL_CONFORMANCES_BY_TYPE_DESCRIPTOR,
+         bool, true,
+         "Enable caching protocol conformances by type descriptor in addition "
+         "to the cache by metadata.")
+
 VARIABLE(SWIFT_DEBUG_CONCURRENCY_ENABLE_COOPERATIVE_QUEUES, bool, true,
         "Enable dispatch cooperative queues in the global executor.")

@@ -70,6 +75,10 @@ VARIABLE(SWIFT_DEBUG_CONCURRENCY_ENABLE_COOPERATIVE_QUEUES, bool, true,
 VARIABLE(SWIFT_DEBUG_RUNTIME_EXCLUSIVITY_LOGGING, bool, false,
         "Enable the an asserts runtime to emit logging as it works.")

+VARIABLE(SWIFT_DEBUG_ENABLE_PROTOCOL_CONFORMANCES_LOOKUP_LOG,
+         bool, false,
+         "Enable logs for the conformance lookup routine.")
+
 #endif

 VARIABLE(SWIFT_BINARY_COMPATIBILITY_VERSION, uint32_t, 0,
--- a/stdlib/public/runtime/ProtocolConformance.cpp
+++ b/stdlib/public/runtime/ProtocolConformance.cpp
@@ -117,6 +117,15 @@ void ProtocolDescriptorFlags::dump() const {

 #endif

+static bool IsDebugLog() {
+#ifndef NDEBUG
+  return runtime::environment::
+      SWIFT_DEBUG_ENABLE_PROTOCOL_CONFORMANCES_LOOKUP_LOG();
+#else
+  return false;
+#endif
+}
+
 #if !defined(NDEBUG) && SWIFT_OBJC_INTEROP
 #include <objc/runtime.h>

@@ -522,16 +531,29 @@ namespace {
  };

  struct ConformanceCacheKey {
-    const Metadata *Type;
+    llvm::PointerUnion<const Metadata *, const TypeContextDescriptor *>
+        TypeOrDescriptor;
    const ProtocolDescriptor *Proto;

    ConformanceCacheKey(const Metadata *type, const ProtocolDescriptor *proto)
-        : Type(type), Proto(proto) {
+        : TypeOrDescriptor(type), Proto(proto) {
      assert(type);
    }

+    ConformanceCacheKey(llvm::PointerUnion<const Metadata *, const TypeContextDescriptor *> typeOrDescriptor, const ProtocolDescriptor *proto)
+        : Proto(proto) {
+      TypeOrDescriptor = typeOrDescriptor;
+      assert(typeOrDescriptor);
+    }
+
+    ConformanceCacheKey(const TypeContextDescriptor *typeDescriptor, const ProtocolDescriptor *proto)
+    : TypeOrDescriptor(typeDescriptor), Proto(proto) {
+      assert(typeDescriptor);
+    }
+
    friend llvm::hash_code hash_value(const ConformanceCacheKey &key) {
-      return llvm::hash_combine(key.Type, key.Proto);
+      return llvm::hash_combine(key.TypeOrDescriptor.getOpaqueValue(),
+                                key.Proto);
    }
  };

@@ -554,28 +576,33 @@ namespace {
      ExtendedStorage *next = nullptr;
    };

-    const Metadata *Type;
+    llvm::PointerUnion<const Metadata *, const TypeContextDescriptor *>
+        TypeOrDescriptor;
    llvm::PointerUnion<const ProtocolDescriptor *, ExtendedStorage *>
        ProtoOrStorage;

-    /// The witness table.
-    const WitnessTable *Witness;
+    union {
+      /// The witness table. Used for type cache records.
+      const WitnessTable *Witness;
+
+      /// The conformance. Used for type descriptor cache records.
+      const ProtocolConformanceDescriptor *Conformance;
+    };

  public:
-    ConformanceCacheEntry(ConformanceCacheKey key,
+    ConformanceCacheEntry(const Metadata *type, const ProtocolDescriptor *proto,
                          ConformanceLookupResult result,
                          std::atomic<ExtendedStorage *> &storageHead)
-      : Type(key.Type), Witness(result.witnessTable)
-    {
+        : TypeOrDescriptor(type), Witness(result.witnessTable) {
      if (!result.globalActorIsolationType) {
-        ProtoOrStorage = key.Proto;
+        ProtoOrStorage = proto;
        return;
      }

      // Allocate extended storage.
      void *memory = malloc(sizeof(ExtendedStorage));
      auto storage = new (memory) ExtendedStorage{
-        key.Proto, result.globalActorIsolationType,
+        proto, result.globalActorIsolationType,
        result.globalActorIsolationWitnessTable
      };

@@ -593,8 +620,17 @@ namespace {
      };
    }

+    ConformanceCacheEntry(const TypeContextDescriptor *typeDescriptor,
+                          const ProtocolDescriptor *proto,
+                          const ProtocolConformanceDescriptor *conformance)
+        : TypeOrDescriptor(typeDescriptor), ProtoOrStorage(proto),
+          Conformance(conformance) {
+      assert(TypeOrDescriptor);
+      assert(ProtoOrStorage);
+    }
+
    bool matchesKey(const ConformanceCacheKey &key) const {
-      return Type == key.Type && getProtocol() == key.Proto;
+      return TypeOrDescriptor == key.TypeOrDescriptor && getProtocol() == key.Proto;
    }

    friend llvm::hash_code hash_value(const ConformanceCacheEntry &entry) {
@@ -614,7 +650,7 @@ namespace {

    /// Get the conformance cache key.
    ConformanceCacheKey getKey() const {
-      return ConformanceCacheKey(Type, getProtocol());
+      return ConformanceCacheKey(TypeOrDescriptor, getProtocol());
    }

    /// Get the cached witness table, or null if we cached failure.
@@ -637,9 +673,14 @@ namespace {
  };
 } // end anonymous namespace

+static bool CanCacheTypeByDescriptor(const TypeContextDescriptor &descriptor) {
+  return descriptor.isGeneric();
+}
+
 // Conformance Cache.
 struct ConformanceState {
-  ConcurrentReadableHashMap<ConformanceCacheEntry> Cache;
+  using CacheType = ConcurrentReadableHashMap<ConformanceCacheEntry>;
+  CacheType Cache;
  ConcurrentReadableArray<ConformanceSection> SectionsToScan;

  /// The head of an intrusive linked list that keeps track of all of the
@@ -647,6 +688,7 @@ struct ConformanceState {
  std::atomic<ConformanceCacheEntry::ExtendedStorage *> ExtendedStorageHead{nullptr};

  bool scanSectionsBackwards;
+  bool envAllowCacheByDescriptors;

 #if USE_DYLD_SHARED_CACHE_CONFORMANCE_TABLES
  uintptr_t dyldSharedCacheStart;
@@ -671,6 +713,8 @@ struct ConformanceState {
  ConformanceState() {
    scanSectionsBackwards =
        runtime::bincompat::useLegacyProtocolConformanceReverseIteration();
+    envAllowCacheByDescriptors = runtime::environment::
+        SWIFT_DEBUG_ENABLE_CACHE_PROTOCOL_CONFORMANCES_BY_TYPE_DESCRIPTOR();

 #if USE_DYLD_SHARED_CACHE_CONFORMANCE_TABLES
    if (__builtin_available(macOS 12.0, iOS 15.0, tvOS 15.0, watchOS 8.0, *)) {
@@ -706,38 +750,62 @@ struct ConformanceState {
  }

  void cacheResult(const Metadata *type, const ProtocolDescriptor *proto,
-                   ConformanceLookupResult result, size_t sectionsCount) {
-    Cache.getOrInsert(ConformanceCacheKey(type, proto),
-                      [&](ConformanceCacheEntry *entry, bool created) {
-                        // Create the entry if needed. If it already exists,
-                        // we're done.
-                        if (!created)
-                          return false;
+                   ConformanceLookupResult result, size_t sectionsCount,
+                   bool allowSaveDescriptor) {
+    CacheType::GetOrInsertManyScope lockedCache(Cache);
+    
+    // Check the current sections count against what was
+    // passed in. If a section count was passed in and they
+    // don't match, then this is not an authoritative entry
+    // and it may have been obsoleted, because the new
+    // sections could contain a conformance in a more
+    // specific type.
+    //
+    // If they DO match, then we can safely add. Another
+    // thread might be adding new sections at this point,
+    // but we will not race with them. That other thread
+    // will add the new sections, then clear the cache.
+    // When it clears the cache, it will block waiting for
+    // this code to complete and relinquish Cache's writer
+    // lock. If we cache a stale entry, it will be
+    // immediately cleared.
+    if (sectionsCount > 0 &&
+      SectionsToScan.snapshot().count() != sectionsCount)
+    return; // abandon the new entry

-                        // Check the current sections count against what was
-                        // passed in. If a section count was passed in and they
-                        // don't match, then this is not an authoritative entry
-                        // and it may have been obsoleted, because the new
-                        // sections could contain a conformance in a more
-                        // specific type.
-                        //
-                        // If they DO match, then we can safely add. Another
-                        // thread might be adding new sections at this point,
-                        // but we will not race with them. That other thread
-                        // will add the new sections, then clear the cache. When
-                        // it clears the cache, it will block waiting for this
-                        // code to complete and relinquish Cache's writer lock.
-                        // If we cache a stale entry, it will be immediately
-                        // cleared.
-                        if (sectionsCount > 0 &&
-                            SectionsToScan.snapshot().count() != sectionsCount)
-                          return false; // abandon the new entry
+    lockedCache.getOrInsert(ConformanceCacheKey(type, proto),
+                            [&](ConformanceCacheEntry *entry, bool created) {
+                              // Create the entry if needed. If it already
+                              // exists, we're done.
+                              if (!created)
+                                return false;

-                        ::new (entry) ConformanceCacheEntry(
-                            ConformanceCacheKey(type, proto), result,
-                            ExtendedStorageHead);
-                        return true; // keep the new entry
-                      });
+                              ::new (entry) ConformanceCacheEntry(
+                                  type, proto, result, ExtendedStorageHead);
+                              return true; // keep the new entry
+                            });
+
+    if (auto typeDescriptor = type->getTypeContextDescriptor();
+        envAllowCacheByDescriptors && allowSaveDescriptor &&
+        typeDescriptor && result.witnessTable &&
+        CanCacheTypeByDescriptor(*typeDescriptor)) {
+#if SWIFT_STDLIB_USE_RELATIVE_PROTOCOL_WITNESS_TABLES
+      auto conformance = lookThroughOptionalConditionalWitnessTable(
+                         reinterpret_cast<const RelativeWitnessTable *>(result.witnessTable))
+                         ->getDescription();
+#else
+      auto conformance = result.witnessTable->getDescription();
+#endif
+      lockedCache.getOrInsert(ConformanceCacheKey(typeDescriptor, proto),
+                              [&](ConformanceCacheEntry *entry, bool created) {
+                                if (!created)
+                                  return false;
+
+                                ::new (entry) ConformanceCacheEntry(
+                                    typeDescriptor, proto, conformance);
+                                return true;
+                              });
+    }
  }

 #ifndef NDEBUG
@@ -858,12 +926,40 @@ swift::swift_registerProtocolConformances(const ProtocolConformanceRecord *begin
  _registerProtocolConformances(C, ConformanceSection{begin, end});
 }

+// Result of `searchInConformanceCache`
+struct SearchInConformanceCacheResult {
+  enum class Source {
+    None,
+    TypeMetadata,
+    TypeDescriptor,
+  };
+
+  /// `IsAuthoritative` is `true` if the result is for the type itself and not a
+  /// superclass. If `false` then we cached a conformance on a superclass, but
+  /// that may be overridden.
+  bool IsAuthoritative;
+  ConformanceLookupResult Result;
+#ifndef NDEBUG
+  Source Source; // For logging purpose
+#endif
+
+  SearchInConformanceCacheResult(bool isAuthoritative,
+                                 ConformanceLookupResult result,
+                                 enum Source source)
+      : IsAuthoritative(isAuthoritative), Result(result)
+#ifndef NDEBUG
+        , Source(source)
+#endif
+  {}
+
+  static SearchInConformanceCacheResult NotFound() {
+    return SearchInConformanceCacheResult(false, {},
+                                          Source::None);
+  }
+};
+
 /// Search for a conformance descriptor in the ConformanceCache.
-/// First element of the return value is `true` if the result is authoritative
-/// i.e. the result is for the type itself and not a superclass. If `false`
-/// then we cached a conformance on a superclass, but that may be overridden.
-/// A return value of `{ false, { } }` indicates nothing was cached.
-static std::pair<bool, ConformanceLookupResult>
+static SearchInConformanceCacheResult
 searchInConformanceCache(const Metadata *type,
                         const ProtocolDescriptor *protocol,
                         bool instantiateSuperclassMetadata) {
@@ -874,13 +970,47 @@ searchInConformanceCache(const Metadata *type,
  MaybeIncompleteSuperclassIterator superclassIterator{
      type, instantiateSuperclassMetadata};
  for (; auto type = superclassIterator.metadata; ++superclassIterator) {
-    if (auto *Value = snapshot.find(ConformanceCacheKey(type, protocol))) {
-      return { type == origType, Value->getResult() };
+    if (auto *cacheEntry = snapshot.find(ConformanceCacheKey(type, protocol))) {
+      return SearchInConformanceCacheResult(
+          type == origType, cacheEntry->getResult(),
+          SearchInConformanceCacheResult::Source::TypeMetadata);
+    }
+    if (auto *typeDescriptor = type->getTypeContextDescriptor();
+        typeDescriptor && CanCacheTypeByDescriptor(*typeDescriptor)) {
+      auto *cacheEntry =
+          snapshot.find(ConformanceCacheKey(typeDescriptor, protocol));
+      if (!cacheEntry)
+        continue;
+      auto conformanceDescriptor = cacheEntry->Conformance;
+      auto result =
+          ConformanceLookupResult::fromConformance(type, conformanceDescriptor);
+      // In case we couldn't get a witness table from the cached conformance
+      // for this type. While it's possible we could find another conformance
+      // that satisfies the requirements, we do NOT attempt to find it.
+      // We cache it and return the result immediatelly.
+      // This aligns with the current logic of the scanning:
+      // When we find a conformance for the given type and protocol we attempt
+      // to get the witness table and cache it and put into `foundWitnesses` no
+      // matter if the witness is nullptr. If we find another conformance in
+      // subsequent iterations we will get the witness and attempt to insert it
+      // into the cache and `foundWitnesses` as well, but both of them will NOT
+      // override the existing entry saved earlier. Later we select the first
+      // witness in `foundWitnesses` in order of hierarchy even if it's null.
+      // See the test case `(GenericSubClass<Int>() as Any as! Hello).hello()`
+      // in `test/multifile/protocol-conformance-redundant.swift` for example.
+      auto sectionsCount = C.SectionsToScan.snapshot().count();
+      bool allowSaveDescriptor = false;
+      C.cacheResult(type, protocol, result, sectionsCount,
+                    allowSaveDescriptor);
+      auto isAuthoritative = origType == type;
+      return SearchInConformanceCacheResult(
+          isAuthoritative, result,
+          SearchInConformanceCacheResult::Source::TypeDescriptor);
    }
  }

  // We did not find a cache entry.
-  return { false, ConformanceLookupResult{} };
+  return SearchInConformanceCacheResult::NotFound();
 }

 /// Get the appropriate context descriptor for a type. If the descriptor is a
@@ -1276,19 +1406,43 @@ swift_conformsToProtocolMaybeInstantiateSuperclasses(
        return {dyldCachedWitnessTable, false};
  }

+  auto debugLogResult = [&](bool found, const char *source) {
+    if (IsDebugLog()) {
+      auto typeName = swift_getTypeName(type, true);
+      const char *status = found ? "found" : "not found";
+      fprintf(stderr, "Check confomance %.*s to %s: %s, source: %s\n",
+             (int)typeName.length, typeName.data, protocol->Name.get(), status,
+             source);
+    }
+  };
+
  // See if we have an authoritative cached conformance. The
  // ConcurrentReadableHashMap data structure allows us to search the map
  // concurrently without locking.
-  auto found =
-      searchInConformanceCache(type, protocol, instantiateSuperclassMetadata);
-  if (found.first) {
+  if (auto cacheSearchResult = searchInConformanceCache(
+          type, protocol, instantiateSuperclassMetadata);
+      cacheSearchResult.IsAuthoritative) {
    // An authoritative negative result can be overridden by a result from dyld.
-    if (!found.second.witnessTable) {
+    if (!cacheSearchResult.Result.witnessTable) {
      if (dyldCachedWitnessTable)
        return {dyldCachedWitnessTable, false};
    }
-
-    return {found.second, false};
+#ifndef NDEBUG
+    const char *source;
+    switch (cacheSearchResult.Source) {
+    case SearchInConformanceCacheResult::Source::None:
+      source = "unknown";
+      break;
+    case SearchInConformanceCacheResult::Source::TypeMetadata:
+      source = "cache by type metadata";
+      break;
+    case SearchInConformanceCacheResult::Source::TypeDescriptor:
+      source = "cache by type descriptor";
+      break;
+    }
+    debugLogResult(cacheSearchResult.Result.witnessTable != nullptr,  source);
+#endif
+    return {cacheSearchResult.Result, false};
  }

  if (dyldCachedConformanceDescriptor) {
@@ -1298,7 +1452,8 @@ swift_conformsToProtocolMaybeInstantiateSuperclasses(
    assert(matchingType);
    auto witness = ConformanceLookupResult::fromConformance(
        matchingType, dyldCachedConformanceDescriptor);
-    C.cacheResult(type, protocol, witness, /*always cache*/ 0);
+    bool allowSaveDescriptor = false; // already have it in the dyld cache
+    C.cacheResult(type, protocol, witness, /*always cache*/ 0, allowSaveDescriptor);
    DYLD_CONFORMANCES_LOG("Caching generic conformance to %s found by DYLD",
                          protocol->Name.get());
    return {witness, false};
@@ -1325,7 +1480,8 @@ swift_conformsToProtocolMaybeInstantiateSuperclasses(
      if (matchingType) {
        auto witness = ConformanceLookupResult::fromConformance(
            matchingType, &descriptor);
-        C.cacheResult(matchingType, protocol, witness, /*always cache*/ 0);
+        bool allowSaveDescriptor = true;
+        C.cacheResult(matchingType, protocol, witness, /*always cache*/ 0, allowSaveDescriptor);
        foundWitnesses.insert({matchingType, witness});
      }
    };
@@ -1358,10 +1514,10 @@ swift_conformsToProtocolMaybeInstantiateSuperclasses(
  MaybeIncompleteSuperclassIterator superclassIterator{
      type, instantiateSuperclassMetadata};
  for (; auto searchType = superclassIterator.metadata; ++superclassIterator) {
-    auto witness = foundWitnesses.lookup(searchType);
-    if (witness) {
+    const auto witnessIt = foundWitnesses.find(searchType);
+    if (witnessIt != foundWitnesses.end()) {
      if (!foundType) {
-        foundWitness = witness;
+        foundWitness = witnessIt->getSecond(); // may be null
        foundType = searchType;
      } else {
        auto foundName = swift_getTypeName(foundType, true);
@@ -1386,13 +1542,16 @@ swift_conformsToProtocolMaybeInstantiateSuperclasses(
    // Do not cache negative results if there were uninstantiated superclasses
    // we didn't search. They might have a conformance that will be found later.
    if (foundWitness || !hasUninstantiatedSuperclass)
-      C.cacheResult(type, protocol, foundWitness, snapshot.count());
+      C.cacheResult(type, protocol, foundWitness, snapshot.count(), /* allowSaveDescriptor */ false);

  // A negative result can be overridden by a result from dyld.
  if (!foundWitness) {
-    if (dyldCachedWitnessTable)
+    if (dyldCachedWitnessTable) {
+      debugLogResult(true, "dyld cache");
      return {dyldCachedWitnessTable, false};
+    }
  }
+  debugLogResult(static_cast<bool>(foundWitness), "section scan");
  return {foundWitness, hasUninstantiatedSuperclass};
 }

--- a/test/Runtime/protocol-conformance-cache-objc.swift
+++ b/test/Runtime/protocol-conformance-cache-objc.swift
@@ -0,0 +1,51 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-build-swift %s -o %t/a.out
+// RUN: env SWIFT_DEBUG_ENABLE_PROTOCOL_CONFORMANCES_LOOKUP_LOG=1 %target-run %t/a.out 2>&1 | %FileCheck %s
+
+// REQUIRES: executable_test
+// REQUIRES: objc_interop
+// REQUIRES: swift_stdlib_asserts
+// UNSUPPORTED: DARWIN_SIMULATOR=ios
+// UNSUPPORTED: DARWIN_SIMULATOR=tvos
+// UNSUPPORTED: DARWIN_SIMULATOR=watchos
+// UNSUPPORTED: DARWIN_SIMULATOR=xros
+// UNSUPPORTED: use_os_stdlib
+
+// The optimizer will remove many of these conformance checks due to statically
+// knowing the result.
+// UNSUPPORTED: swift_test_mode_optimize
+// UNSUPPORTED: swift_test_mode_optimize_size
+
+import Foundation
+
+protocol Proto {}
+extension Proto {
+  static var selfType: Any.Type { Self.self }
+}
+
+func conformsToProto<T>(_ type: T.Type) -> Bool {
+  (type as? Proto.Type)?.selfType == type
+}
+
+func doesNotConformToProto<T>(_ type: T) -> Bool {
+  (type as? Proto.Type) == nil
+}
+
+@objc class BaseClass: NSObject, Proto {}
+@objc class DerivedClass: BaseClass {}
+@objc class NonConformingClass: NSObject {}
+
+// CHECK: Check confomance a.BaseClass to Proto: found, source: section scan
+assert(conformsToProto(BaseClass.self))
+// CHECK: Check confomance a.BaseClass to Proto: found, source: cache by type metadata
+assert(conformsToProto(BaseClass.self))
+
+// CHECK: Check confomance a.DerivedClass to Proto: found, source: section scan
+assert(conformsToProto(DerivedClass.self))
+// CHECK: Check confomance a.DerivedClass to Proto: found, source: cache by type metadata
+assert(conformsToProto(DerivedClass.self))
+
+// CHECK: Check confomance a.NonConformingClass to Proto: not found, source: section scan
+assert(doesNotConformToProto(NonConformingClass.self))
+// CHECK: Check confomance a.NonConformingClass to Proto: not found, source: cache by type metadata
+assert(doesNotConformToProto(NonConformingClass.self))
--- a/test/Runtime/protocol-conformance-cache.swift
+++ b/test/Runtime/protocol-conformance-cache.swift
@@ -0,0 +1,121 @@
+// RUN: %empty-directory(%t)
+// RUN: %target-build-swift %s -o %t/a.out
+// RUN: env SWIFT_DEBUG_ENABLE_PROTOCOL_CONFORMANCES_LOOKUP_LOG=1 %target-run %t/a.out 2>&1 | %FileCheck %s
+
+// REQUIRES: executable_test
+// REQUIRES: swift_stdlib_asserts
+// UNSUPPORTED: DARWIN_SIMULATOR=ios
+// UNSUPPORTED: DARWIN_SIMULATOR=tvos
+// UNSUPPORTED: DARWIN_SIMULATOR=watchos
+// UNSUPPORTED: DARWIN_SIMULATOR=xros
+// UNSUPPORTED: use_os_stdlib
+
+// The optimizer will remove many of these conformance checks due to statically
+// knowing the result.
+// UNSUPPORTED: swift_test_mode_optimize
+// UNSUPPORTED: swift_test_mode_optimize_size
+
+protocol Proto {}
+extension Proto {
+  static var selfType: Any.Type { Self.self }
+}
+
+func conformsToProto<T>(_ type: T.Type) -> Bool {
+  (type as? Proto.Type)?.selfType == type
+}
+
+func doesNotConformToProto<T>(_ type: T) -> Bool {
+  (type as? Proto.Type) == nil
+}
+
+extension Array: Proto {}
+extension Dictionary: Proto where Key: Proto {}
+extension Int: Proto {}
+
+// CHECK: Check confomance Swift.Int to Proto: found, source: section scan
+assert(conformsToProto(Int.self))
+// CHECK: Check confomance Swift.Int to Proto: found, source: cache by type metadata
+assert(conformsToProto(Int.self))
+
+// CHECK: Check confomance Swift.String to Proto: not found, source: section scan
+assert(doesNotConformToProto(String.self))
+// CHECK: Check confomance Swift.String to Proto: not found, source: cache by type metadata
+assert(doesNotConformToProto(String.self))
+
+// CHECK: Check confomance Swift.Array<Swift.Int> to Proto: found, source: section scan
+assert(conformsToProto([Int].self))
+// CHECK: Check confomance Swift.Array<Swift.Int> to Proto: found, source: cache by type metadata
+assert(conformsToProto([Int].self))
+
+// CHECK: Check confomance Swift.Array<Swift.String> to Proto: found, source: cache by type descriptor
+assert(conformsToProto([String].self))
+// CHECK: Check confomance Swift.Array<Swift.String> to Proto: found, source: cache by type metadata
+assert(conformsToProto([String].self))
+
+// CHECK: Check confomance Swift.Dictionary<Swift.Int, Swift.Int> to Proto: found, source: section scan
+assert(conformsToProto([Int: Int].self))
+
+// CHECK: Check confomance Swift.Dictionary<Swift.String, Swift.Int> to Proto: not found, source: cache by type descriptor
+assert(doesNotConformToProto([String: Int].self))
+// CHECK: Check confomance Swift.Dictionary<Swift.String, Swift.Int> to Proto: not found, source: cache by type metadata
+assert(doesNotConformToProto([String: Int].self))
+
+
+class BaseClass: Proto {}
+class DerivedClass: BaseClass {}
+class GenericClass<T>: Proto {}
+class GenericClassConditional<T> {}
+extension GenericClassConditional: Proto where T: Proto {}
+
+// CHECK: Check confomance a.BaseClass to Proto: found, source: section scan
+assert(conformsToProto(BaseClass.self))
+// CHECK: Check confomance a.BaseClass to Proto: found, source: cache by type metadata
+assert(conformsToProto(BaseClass.self))
+
+// CHECK: Check confomance a.DerivedClass to Proto: found, source: section scan
+assert(conformsToProto(DerivedClass.self))
+// CHECK: Check confomance a.DerivedClass to Proto: found, source: cache by type metadata
+assert(conformsToProto(DerivedClass.self))
+
+// CHECK: Check confomance a.GenericClass<Swift.Int> to Proto: found, source: section scan
+assert(conformsToProto(GenericClass<Int>.self))
+// CHECK: Check confomance a.GenericClass<Swift.Int> to Proto: found, source: cache by type metadata
+assert(conformsToProto(GenericClass<Int>.self))
+
+// CHECK: Check confomance a.GenericClass<Swift.String> to Proto: found, source: cache by type descriptor
+assert(conformsToProto(GenericClass<String>.self))
+// CHECK: Check confomance a.GenericClass<Swift.String> to Proto: found, source: cache by type metadata
+assert(conformsToProto(GenericClass<String>.self))
+
+// CHECK: Check confomance a.GenericClassConditional<Swift.Int> to Proto: found, source: section scan
+assert(conformsToProto(GenericClassConditional<Int>.self))
+// CHECK: Check confomance a.GenericClassConditional<Swift.Int> to Proto: found, source: cache by type metadata
+assert(conformsToProto(GenericClassConditional<Int>.self))
+
+// CHECK: Check confomance a.GenericClassConditional<Swift.String> to Proto: not found, source: cache by type descriptor
+assert(doesNotConformToProto(GenericClassConditional<String>.self))
+// CHECK: Check confomance a.GenericClassConditional<Swift.String> to Proto: not found, source: cache by type metadata
+assert(doesNotConformToProto(GenericClassConditional<String>.self))
+
+enum Enum: Proto {}
+extension Optional: Proto where Wrapped: Proto {}
+
+// CHECK: Check confomance a.Enum to Proto: found, source: section scan
+assert(conformsToProto(Enum.self))
+// CHECK: Check confomance a.Enum to Proto: found, source: cache by type metadata
+assert(conformsToProto(Enum.self))
+
+// CHECK: Check confomance Swift.Optional<a.Enum> to Proto: found, source: section scan
+assert(conformsToProto(Enum?.self))
+// CHECK: Check confomance Swift.Optional<a.Enum> to Proto: found, source: cache by type metadata
+assert(conformsToProto(Enum?.self))
+
+// CHECK: Check confomance Swift.Optional<Swift.Int> to Proto: found, source: cache by type descriptor
+assert(conformsToProto(Int?.self))
+// CHECK: Check confomance Swift.Optional<Swift.Int> to Proto: found, source: cache by type metadata
+assert(conformsToProto(Int?.self))
+
+// CHECK: Check confomance Swift.Optional<Swift.String> to Proto: not found, source: cache by type descriptor
+assert(doesNotConformToProto(String?.self))
+// CHECK: Check confomance Swift.Optional<Swift.String> to Proto: not found, source: cache by type metadata
+assert(doesNotConformToProto(String?.self))
--- a/test/multifile/Inputs/protocol-conformance-redundant-def.swift
+++ b/test/multifile/Inputs/protocol-conformance-redundant-def.swift
@@ -5,3 +5,11 @@ public protocol Hello {
 open class Super {
  public init() {}
 }
+
+open class GenericSuperClass<T> {
+  public init() {}
+}
+
+public struct GenericStruct<T> {
+  public init() {}
+}
--- a/test/multifile/Inputs/protocol-conformance-redundant-ext.swift
+++ b/test/multifile/Inputs/protocol-conformance-redundant-ext.swift
@@ -1,7 +1,19 @@
 import Def

-extension Super : Hello {
+extension Super: @retroactive Hello {
  public func hello() {
-    print("Hello")
+    print("Hello from Ext")
+  }
+}
+
+extension GenericSuperClass: @retroactive Hello {
+  public func hello() {
+    print("Hello from Ext")
+  }
+}
+
+extension GenericStruct: @retroactive Hello {
+  public func hello() {
+    print("Hello from Ext")
  }
 }
--- a/test/multifile/protocol-conformance-redundant.swift
+++ b/test/multifile/protocol-conformance-redundant.swift
@@ -3,14 +3,11 @@
 // RUN: %target-build-swift-dylib(%t/%target-library-name(Ext)) -module-name Ext -emit-module -emit-module-path %t/Ext.swiftmodule -I%t -L%t -lDef %S/Inputs/protocol-conformance-redundant-ext.swift
 // RUN: %target-build-swift -I%t -L%t -lDef -o %t/main %target-rpath(%t) %s
 // RUN: %target-codesign %t/main %t/%target-library-name(Def) %t/%target-library-name(Ext)
-// RUN: %target-run %t/main %t/%target-library-name(Def) %t/%target-library-name(Ext) 2>&1 | %FileCheck %s
+// RUN: %target-run %t/main %t/%target-library-name(Def) %t/%target-library-name(Ext) 2> >(%FileCheck %s -check-prefix=CHECK-STDERR) | %FileCheck %s

 // REQUIRES: executable_test
 // XFAIL: OS=windows-msvc

-// CHECK: Warning: 'main.Sub' conforms to protocol 'Hello', but it also inherits conformance from 'Def.Super'.  Relying on a particular conformance is undefined behaviour.
-// CHECK: Hello
-
 import StdlibUnittest

 #if canImport(Darwin)
@@ -35,7 +32,40 @@ class Sub : Super, Hello {
  }
 }

+// CHECK-STDERR: Warning: 'main.Sub' conforms to protocol 'Hello', but it also inherits conformance from 'Def.Super'.  Relying on a particular conformance is undefined behaviour.
 let s = Sub() as AnyObject as! Hello
-
+// CHECK: Hello
 s.hello()

+extension GenericStruct: @retroactive Hello where T == String {
+  public func hello() {
+    print("Hello from main")
+  }
+}
+
+// CHECK-STDERR: Warning: 'main.GenericSubClass<Swift.String>' conforms to protocol 'Hello', but it also inherits conformance from 'Def.GenericSuperClass<Swift.String>'. Relying on a particular conformance is undefined behaviour.
+// CHECK: Hello from main
+(GenericStruct<String>() as Any as! Hello).hello()
+
+assert(GenericStruct<Int>() as Any as? Hello == nil)
+
+class GenericSubClass<T>: GenericSuperClass<T> {}
+extension GenericSubClass: Hello where T == String {
+  func hello() {
+    print("Hello from main")
+  }
+}
+
+// CHECK: Hello from main
+(GenericSubClass<String>() as Any as! Hello).hello()
+
+// https://github.com/swiftlang/swift/issues/82889
+// CHECK: Expected nil
+// CHECK: Expected nil
+for _ in 0..<2 {
+  if GenericSubClass<Int>() as Any as? Hello != nil {
+    print("Unexpected successful cast")
+  } else {
+    print("Expected nil")
+  }
+}