Skip to content

Commit 885f829

Browse files
committed
[Conformance cache] Add allocated extended-storage for entries to the free list
This memory is part of the conformance cache concurrent hash map, so when we clear the conformance cache, record each of the allocated pointers within the concurrent map's free list. This way, it'll be freed with the rest of the concurrent map when it's safe to do so.
1 parent d576fa3 commit 885f829

File tree

2 files changed

+50
-8
lines changed

2 files changed

+50
-8
lines changed

include/swift/Runtime/Concurrent.h

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -750,7 +750,12 @@ struct ConcurrentReadableHashMap {
750750

751751
/// Clear the hash table, freeing (when safe) all memory currently used for
752752
/// indices and elements.
753-
void clear() {
753+
///
754+
/// - addFreedNodes: if nonempty, this function will be called with the head
755+
/// of the free list. The function may add additional nodes to the free
756+
/// list, which will be freed when it is safe to do so.
757+
void clear(std::function<void(ConcurrentFreeListNode *&)> addFreedNodes
758+
= nullptr) {
754759
typename MutexTy::ScopedLock guard(WriterLock);
755760

756761
IndexStorage indices = Indices.load(std::memory_order_relaxed);
@@ -766,6 +771,9 @@ struct ConcurrentReadableHashMap {
766771
ConcurrentFreeListNode::add(&FreeList, ptr);
767772
ConcurrentFreeListNode::add(&FreeList, elements);
768773

774+
if (addFreedNodes)
775+
addFreedNodes(FreeList);
776+
769777
deallocateFreeListIfSafe();
770778
}
771779
};

stdlib/public/runtime/ProtocolConformance.cpp

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -536,7 +536,7 @@ namespace {
536536
};
537537

538538
struct ConformanceCacheEntry {
539-
private:
539+
public:
540540
/// Storage used when we have global actor isolation on the conformance.
541541
struct ExtendedStorage {
542542
/// The protocol to which the type conforms.
@@ -549,18 +549,22 @@ namespace {
549549
/// When the conformance is global-actor-isolated, this is the conformance
550550
/// of globalActorIsolationType to GlobalActor.
551551
const WitnessTable *globalActorIsolationWitnessTable = nullptr;
552+
553+
/// The next pointer in the list of extended storage allocations.
554+
ExtendedStorage *next = nullptr;
552555
};
553556

554557
const Metadata *Type;
555-
llvm::PointerUnion<const ProtocolDescriptor *, const ExtendedStorage *>
558+
llvm::PointerUnion<const ProtocolDescriptor *, ExtendedStorage *>
556559
ProtoOrStorage;
557560

558561
/// The witness table.
559562
const WitnessTable *Witness;
560563

561564
public:
562565
ConformanceCacheEntry(ConformanceCacheKey key,
563-
ConformanceLookupResult result)
566+
ConformanceLookupResult result,
567+
std::atomic<ExtendedStorage *> &storageHead)
564568
: Type(key.Type), Witness(result.witnessTable)
565569
{
566570
if (!result.globalActorIsolationType) {
@@ -576,6 +580,17 @@ namespace {
576580
};
577581

578582
ProtoOrStorage = storage;
583+
584+
// Add the storage pointer to the list of extended storage allocations
585+
// so that we can free them later.
586+
auto head = storageHead.load(std::memory_order_relaxed);
587+
while (true) {
588+
storage->next = head;
589+
if (storageHead.compare_exchange_weak(
590+
head, storage, std::memory_order_release,
591+
std::memory_order_relaxed))
592+
break;
593+
};
579594
}
580595

581596
bool matchesKey(const ConformanceCacheKey &key) const {
@@ -591,7 +606,7 @@ namespace {
591606
if (auto proto = ProtoOrStorage.dyn_cast<const ProtocolDescriptor *>())
592607
return proto;
593608

594-
if (auto storage = ProtoOrStorage.dyn_cast<const ExtendedStorage *>())
609+
if (auto storage = ProtoOrStorage.dyn_cast<ExtendedStorage *>())
595610
return storage->Proto;
596611

597612
return nullptr;
@@ -611,7 +626,7 @@ namespace {
611626
if (ProtoOrStorage.is<const ProtocolDescriptor *>())
612627
return ConformanceLookupResult { Witness, nullptr, nullptr };
613628

614-
if (auto storage = ProtoOrStorage.dyn_cast<const ExtendedStorage *>()) {
629+
if (auto storage = ProtoOrStorage.dyn_cast<ExtendedStorage *>()) {
615630
return ConformanceLookupResult(
616631
Witness, storage->globalActorIsolationType,
617632
storage->globalActorIsolationWitnessTable);
@@ -626,6 +641,11 @@ namespace {
626641
struct ConformanceState {
627642
ConcurrentReadableHashMap<ConformanceCacheEntry> Cache;
628643
ConcurrentReadableArray<ConformanceSection> SectionsToScan;
644+
645+
/// The head of an intrusive linked list that keeps track of all of the
646+
/// conformance cache entries that require extended storage.
647+
std::atomic<ConformanceCacheEntry::ExtendedStorage *> ExtendedStorageHead{nullptr};
648+
629649
bool scanSectionsBackwards;
630650

631651
#if USE_DYLD_SHARED_CACHE_CONFORMANCE_TABLES
@@ -714,7 +734,8 @@ struct ConformanceState {
714734
return false; // abandon the new entry
715735

716736
::new (entry) ConformanceCacheEntry(
717-
ConformanceCacheKey(type, proto), result);
737+
ConformanceCacheKey(type, proto), result,
738+
ExtendedStorageHead);
718739
return true; // keep the new entry
719740
});
720741
}
@@ -748,7 +769,20 @@ static void _registerProtocolConformances(ConformanceState &C,
748769

749770
// Blow away the conformances cache to get rid of any negative entries that
750771
// may now be obsolete.
751-
C.Cache.clear();
772+
C.Cache.clear([&](ConcurrentFreeListNode *&freeListHead) {
773+
// The extended storage for conformance entries will need to be freed
774+
// eventually. Put it on the concurrent free list so the cache will do so.
775+
auto storageHead = C.ExtendedStorageHead.load(std::memory_order_relaxed);
776+
while (storageHead) {
777+
auto current = storageHead;
778+
auto newHead = current->next;
779+
if (C.ExtendedStorageHead.compare_exchange_weak(
780+
storageHead, newHead, std::memory_order_release,
781+
std::memory_order_relaxed)) {
782+
ConcurrentFreeListNode::add(&freeListHead, current);
783+
}
784+
}
785+
});
752786
}
753787

754788
void swift::addImageProtocolConformanceBlockCallbackUnsafe(

0 commit comments

Comments
 (0)