[Runtime] Move ConcurrentReadableArray's allocate/deallocate functions into Storage. Mark ConcurrentReadableArray as un-copyable, un-assignable, and un-movable. Use SWIFT_MEMORY_ORDER_CONSUME instead of std::memory_order_consume. Make read() pass a const pointer.

rdar://problem/37173156

Author: mikeash

include/swift/Runtime/Atomic.h

@@ -0,0 +1,31 @@
+//===--- Atomic.h - Utilities for atomic operations. ------------*- C++ -*-===//
+//
+// This source file is part of the Swift.org open source project
+//
+// Copyright (c) 2014 - 2018 Apple Inc. and the Swift project authors
+// Licensed under Apache License v2.0 with Runtime Library Exception
+//
+// See https://swift.org/LICENSE.txt for license information
+// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
+//
+//===----------------------------------------------------------------------===//
+//
+// Utilities for atomic operations, to use with std::atomic.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef SWIFT_RUNTIME_ATOMIC_H
+#define SWIFT_RUNTIME_ATOMIC_H
+
+// FIXME: Workaround for rdar://problem/18889711. 'Consume' does not require
+// a barrier on ARM64, but LLVM doesn't know that. Although 'relaxed'
+// is formally UB by C++11 language rules, we should be OK because neither
+// the processor model nor the optimizer can realistically reorder our uses
+// of 'consume'.
+#if __arm64__ || __arm__
+#  define SWIFT_MEMORY_ORDER_CONSUME (std::memory_order_relaxed)
+#else
+#  define SWIFT_MEMORY_ORDER_CONSUME (std::memory_order_consume)
+#endif
+
+#endif

include/swift/Runtime/Concurrent.h

@@ -17,6 +17,7 @@
 #include <functional>
 #include <stdint.h>
 #include "llvm/Support/Allocator.h"
+#include "Atomic.h"
 #include "Debug.h"
 #include "Mutex.h"
 
@@ -422,7 +423,22 @@ template <class ElemTy> struct ConcurrentReadableArray {
   struct Storage {
     std::atomic<size_t> Count;
     typename std::aligned_storage<sizeof(ElemTy), alignof(ElemTy)>::type Elem;
-    
+
+    static Storage *allocate(size_t capacity) {
+      auto size = sizeof(Storage) + (capacity - 1) * sizeof(Storage().Elem);
+      auto *ptr = reinterpret_cast<Storage *>(malloc(size));
+      if (!ptr) swift::crash("Could not allocate memory.");
+      ptr->Count.store(0, std::memory_order_relaxed);
+      return ptr;
+    }
+
+    void deallocate() {
+      for (size_t i = 0; i < Count; i++) {
+        data()[i].~ElemTy();
+      }
+      free(this);
+    }
+
     ElemTy *data() {
       return reinterpret_cast<ElemTy *>(&Elem);
     }
@@ -434,33 +450,22 @@ template <class ElemTy> struct ConcurrentReadableArray {
   Mutex WriterLock;
   std::vector<Storage *> FreeList;
 
-  Storage *allocate(size_t capacity) {
-    auto size = sizeof(Storage) + (capacity - 1) * sizeof(Storage().Elem);
-    auto *ptr = reinterpret_cast<Storage *>(malloc(size));
-    if (!ptr) swift::crash("Could not allocate memory.");
-    ptr->Count.store(0, std::memory_order_relaxed);
-    return ptr;
-  }
+public:
+  // This type cannot be safely copied, moved, or deleted.
+  ConcurrentReadableArray(const ConcurrentReadableArray &) = delete;
+  ConcurrentReadableArray(ConcurrentReadableArray &&) = delete;
+  ConcurrentReadableArray &operator=(const ConcurrentReadableArray &) = delete;
 
-  void deallocate(Storage *storage) {
-    if (storage == nullptr) return;
-    
-    auto *data = storage->data();
-    for (size_t i = 0; i < storage->Count; i++) {
-      data[i].~ElemTy();
-    }
-    free(storage);
-  }
+  ConcurrentReadableArray() : Capacity(0), ReaderCount(0), Elements(nullptr) {}
 
-public:
   void push_back(const ElemTy &elem) {
     ScopedLock guard(WriterLock);
 
     auto *storage = Elements.load(std::memory_order_relaxed);
     auto count = storage ? storage->Count.load(std::memory_order_relaxed) : 0;
     if (count >= Capacity) {
       auto newCapacity = std::max((size_t)16, count * 2);
-      auto *newStorage = allocate(newCapacity);
+      auto *newStorage = Storage::allocate(newCapacity);
       if (storage) {
         std::copy(storage->data(), storage->data() + count, newStorage->data());
         newStorage->Count.store(count, std::memory_order_relaxed);
@@ -477,7 +482,7 @@ template <class ElemTy> struct ConcurrentReadableArray {
 
     if (ReaderCount.load(std::memory_order_acquire) == 0)
       for (Storage *storage : FreeList)
-        deallocate(storage);
+        storage->deallocate();
   }
 
   /// Read the contents of the array. The parameter `f` is called with
@@ -486,9 +491,9 @@ template <class ElemTy> struct ConcurrentReadableArray {
   /// `read` was called. The pointer becomes invalid after `f` returns.
   template <class F> auto read(F f) -> decltype(f(nullptr, 0)) {
     ReaderCount.fetch_add(1, std::memory_order_acquire);
-    auto *storage = Elements.load(std::memory_order_consume);
+    auto *storage = Elements.load(SWIFT_MEMORY_ORDER_CONSUME);
     auto count = storage->Count.load(std::memory_order_acquire);
-    auto *ptr = storage->data();
+    const auto *ptr = storage->data();
 
     decltype(f(nullptr, 0)) result = f(ptr, count);
 
@@ -499,7 +504,7 @@ template <class ElemTy> struct ConcurrentReadableArray {
 
   /// Get the current count. It's just a snapshot and may be obsolete immediately.
   size_t count() {
-    return read([](ElemTy *ptr, size_t count) -> size_t {
+    return read([](const ElemTy *ptr, size_t count) -> size_t {
       return count;
     });
   }

stdlib/public/SwiftShims/RefCount.h

@@ -35,19 +35,10 @@ typedef InlineRefCountsPlaceholder InlineRefCounts;
 
 #include "llvm/Support/Compiler.h"
 #include "swift/Basic/type_traits.h"
+#include "swift/Runtime/Atomic.h"
 #include "swift/Runtime/Config.h"
 #include "swift/Runtime/Debug.h"
 
-// FIXME: Workaround for rdar://problem/18889711. 'Consume' does not require
-// a barrier on ARM64, but LLVM doesn't know that. Although 'relaxed'
-// is formally UB by C++11 language rules, we should be OK because neither
-// the processor model nor the optimizer can realistically reorder our uses
-// of 'consume'.
-#if __arm64__ || __arm__
-#  define SWIFT_MEMORY_ORDER_CONSUME (std::memory_order_relaxed)
-#else
-#  define SWIFT_MEMORY_ORDER_CONSUME (std::memory_order_consume)
-#endif
 
 /*
   An object conceptually has three refcounts. These refcounts 

stdlib/public/runtime/ProtocolConformance.cpp

@@ -326,7 +326,7 @@ void ConformanceState::verify() const {
   // Iterate over all of the sections and verify all of the protocol
   // descriptors.
   auto &Self = const_cast<ConformanceState &>(*this);
-  Self.SectionsToScan.read([](ConformanceSection *ptr, size_t count) -> char {
+  Self.SectionsToScan.read([](const ConformanceSection *ptr, size_t count) -> char {
     for (size_t i = 0; i < count; i++) {
       for (const auto &Record : ptr[i]) {
         Record.get()->verify();
@@ -548,7 +548,7 @@ swift_conformsToProtocolImpl(const Metadata * const type,
   // Prepare to scan conformance records.
   size_t scannedCount;
   auto returnNull = C.SectionsToScan
-    .read([&](ConformanceSection *ptr, size_t count) -> bool {
+    .read([&](const ConformanceSection *ptr, size_t count) -> bool {
     scannedCount = count;
     // Scan only sections that were not scanned yet.
     // If we found an out-of-date negative cache entry,
@@ -658,7 +658,7 @@ swift::_searchConformancesByMangledTypeName(Demangle::NodePointer node) {
   auto &C = Conformances.get();
 
   return C.SectionsToScan
-    .read([&](ConformanceSection *ptr, size_t count) -> const TypeContextDescriptor * {
+    .read([&](const ConformanceSection *ptr, size_t count) -> const TypeContextDescriptor * {
     for (size_t i = 0; i < count; i++) {
       auto &section = ptr[i];
       for (const auto &record : section) {