Skip to content

[Runtime] Move ElementCapacity into the Elements allocation. #34464

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

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 39 additions & 22 deletions include/swift/Runtime/Concurrent.h
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ template <class ElemTy> struct ConcurrentReadableHashMap {
/// rounds up to the next allocation quantum by calling `malloc_good_size`.
/// Otherwise, just return the passed-in size, which is always valid even if
/// not necessarily optimal.
size_t goodSize(size_t size) {
static size_t goodSize(size_t size) {
#if defined(__APPLE__) && defined(__MACH__)
return malloc_good_size(size);
#else
Expand Down Expand Up @@ -693,6 +693,29 @@ template <class ElemTy> struct ConcurrentReadableHashMap {
}
};

/// The struct used for element storage. The `Elem` member is considered to be
/// the first element of a variable-length array, whose size is determined by
/// the allocation.
struct ElementStorage {
uint32_t Capacity;
ElemTy Elem;

static ElementStorage *allocate(size_t capacity) {
auto headerSize = offsetof(ElementStorage, Elem);
auto size = goodSize(headerSize + capacity * sizeof(ElemTy));

auto *ptr = reinterpret_cast<ElementStorage *>(malloc(size));
if (!ptr)
swift::crash("Could not allocate memory.");

ptr->Capacity = (size - headerSize) / sizeof(ElemTy);

return ptr;
}

ElemTy *data() { return &Elem; }
};

/// A simple linked list representing pointers that need to be freed.
struct FreeListNode {
FreeListNode *Next;
Expand Down Expand Up @@ -723,17 +746,14 @@ template <class ElemTy> struct ConcurrentReadableHashMap {
std::atomic<uint32_t> ElementCount{0};

/// The array of elements.
std::atomic<ElemTy *> Elements{nullptr};
std::atomic<ElementStorage *> Elements{nullptr};

/// The array of indices.
std::atomic<IndexStorage *> Indices{nullptr};

/// The writer lock, which must be taken before any mutation of the table.
StaticMutex WriterLock;

/// The maximum number of elements that the current elements array can hold.
uint32_t ElementCapacity{0};

/// The list of pointers to be freed once no readers are active.
FreeListNode *FreeList{nullptr};

Expand All @@ -754,22 +774,18 @@ template <class ElemTy> struct ConcurrentReadableHashMap {

/// Grow the elements array, adding the old array to the free list and
/// returning the new array with all existing elements copied into it.
ElemTy *resize(ElemTy *elements, size_t elementCount) {
ElementStorage *resize(ElementStorage *elements, size_t elementCount) {
// Grow capacity by 25%, making sure we grow by at least 1.
size_t newCapacity =
std::max(elementCount + (elementCount >> 2), elementCount + 1);
size_t newSize = newCapacity * sizeof(ElemTy);

newSize = goodSize(newSize);
newCapacity = newSize / sizeof(ElemTy);
auto *newElements = ElementStorage::allocate(newCapacity);

ElemTy *newElements = static_cast<ElemTy *>(malloc(newSize));
if (elements) {
memcpy(newElements, elements, elementCount * sizeof(ElemTy));
memcpy(newElements->data(), elements->data(),
elementCount * sizeof(ElemTy));
FreeListNode::add(&FreeList, elements);
}

ElementCapacity = newCapacity;
Elements.store(newElements, std::memory_order_release);
return newElements;
}
Expand Down Expand Up @@ -919,16 +935,17 @@ template <class ElemTy> struct ConcurrentReadableHashMap {
// should not happen often.
IndexStorage *indices;
size_t elementCount;
ElemTy *elements;
ElemTy *elements2;
ElementStorage *elements;
ElementStorage *elements2;
do {
elements = Elements.load(std::memory_order_acquire);
indices = Indices.load(std::memory_order_acquire);
elementCount = ElementCount.load(std::memory_order_acquire);
elements2 = Elements.load(std::memory_order_acquire);
} while (elements != elements2);

return Snapshot(this, indices, elements, elementCount);
ElemTy *elementsPtr = elements ? elements->data() : nullptr;
return Snapshot(this, indices, elementsPtr, elementCount);
}

/// Get an element by key, or insert a new element for that key if one is not
Expand Down Expand Up @@ -958,8 +975,9 @@ template <class ElemTy> struct ConcurrentReadableHashMap {
auto indicesCapacityLog2 = indices->CapacityLog2;
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, elements);
auto found = this->find(key, indices, elementCount, elementsPtr);
if (found.first) {
call(found.first, false);
deallocateFreeListIfSafe();
Expand All @@ -973,15 +991,15 @@ template <class ElemTy> struct ConcurrentReadableHashMap {
auto emptyCount = indicesCapacity - (elementCount + 1);
auto proportion = indicesCapacity / emptyCount;
if (proportion >= ResizeProportion) {
indices = resize(indices, indicesCapacityLog2, elements);
found = find(key, indices, elementCount, elements);
indices = resize(indices, indicesCapacityLog2, elementsPtr);
found = find(key, indices, elementCount, elementsPtr);
assert(!found.first && "Shouldn't suddenly find the key after rehashing");
}

if (elementCount >= ElementCapacity) {
if (!elements || elementCount >= elements->Capacity) {
elements = resize(elements, elementCount);
}
auto *element = &elements[elementCount];
auto *element = &elements->data()[elementCount];

// Order matters: fill out the element, then update the count,
// then update the index.
Expand Down Expand Up @@ -1010,7 +1028,6 @@ template <class ElemTy> struct ConcurrentReadableHashMap {
Indices.store(nullptr, std::memory_order_relaxed);
ElementCount.store(0, std::memory_order_relaxed);
Elements.store(nullptr, std::memory_order_relaxed);
ElementCapacity = 0;

FreeListNode::add(&FreeList, indices);
FreeListNode::add(&FreeList, elements);
Expand Down
2 changes: 2 additions & 0 deletions test/stdlib/symbol-visibility-linux.test-sh
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
// RUN: -e _ZSt16__once_call_implISt12_Bind_simpleIFPFvPvEPA80_cEEEvv \
// RUN: -e _ZSt16__once_call_implISt12_Bind_simpleIFPFvPvEPA88_cEEEvv \
// RUN: -e _ZSt16__once_call_implISt12_Bind_simpleIFPFvPvEPA104_cEEEvv \
// RUN: -e _ZSt16__once_call_implISt12_Bind_simpleIFPFvPvEPA160_cEEEvv \
// RUN: -e _ZSt16__once_call_implISt12_Bind_simpleIFPFvPvEPA168_cEEEvv \
// RUN: -e _ZSt16__once_call_implISt12_Bind_simpleIFPFvPvEPA216_cEEEvv \
// RUN: -e _ZN9__gnu_cxx12__to_xstringISscEET_PFiPT0_mPKS2_P13__va_list_tagEmS5_z \
Expand All @@ -56,6 +57,7 @@
// RUN: -e _ZSt16__once_call_implISt12_Bind_simpleIFPFvPvEPA80_cEEEvv \
// RUN: -e _ZSt16__once_call_implISt12_Bind_simpleIFPFvPvEPA88_cEEEvv \
// RUN: -e _ZSt16__once_call_implISt12_Bind_simpleIFPFvPvEPA104_cEEEvv \
// RUN: -e _ZSt16__once_call_implISt12_Bind_simpleIFPFvPvEPA160_cEEEvv \
// RUN: -e _ZSt16__once_call_implISt12_Bind_simpleIFPFvPvEPA168_cEEEvv \
// RUN: -e _ZSt16__once_call_implISt12_Bind_simpleIFPFvPvEPA216_cEEEvv \
// RUN: -e _ZN9__gnu_cxx12__to_xstringISscEET_PFiPT0_mPKS2_P13__va_list_tagEmS5_z \
Expand Down