Simplify and optimize the structural type metadata caches to use ConcurrentMap directly.

Previously, these were all using MetadataCache.  MetadataCache is a
more heavyweight structure which acquires a lock before building the
metadata.  This is appropriate if building the metadata is very
expensive or might have semantic side-effects which cannot be rolled
back.  It's also useful when there's a risk of re-entrance, since it
can diagnose such things instead of simply dead-locking or infinitely
recursing.  However, it's necessary for structural cases like tuple
and function types, and instead we can just use ConcurrentMap, which
does a compare-and-swap to publish the constructed metadata and
potentially destroys it if another thread successfully won the race.

This is an optimization which we could not previously attempt.

As part of this, fix tuple metadata uniquing to consider the label
string correctly.  This exposes a bug where the runtime demangling
of tuple metadata nodes doesn't preserve labels; fix this as well.

Author: rjmccall

include/swift/Runtime/Concurrent.h

@@ -19,6 +19,8 @@
 #include <stdio.h>
 #endif
 
+namespace swift {
+
 /// This is a node in a concurrent linked list.
 template <class ElemTy> struct ConcurrentListNode {
   ConcurrentListNode(ElemTy Elem) : Payload(Elem), Next(nullptr) {}
@@ -123,6 +125,29 @@ template <class ElemTy> struct ConcurrentList {
   std::atomic<ConcurrentListNode<ElemTy> *> First;
 };
 
+template <class T, bool Delete> class AtomicMaybeOwningPointer;
+
+template <class T>
+class AtomicMaybeOwningPointer<T, false> {
+public:
+  std::atomic<T*> Value;
+  constexpr AtomicMaybeOwningPointer(T *value) : Value(value) {}
+};
+
+template <class T>
+class AtomicMaybeOwningPointer<T, true> {
+public:
+  std::atomic<T*> Value;
+  constexpr AtomicMaybeOwningPointer(T *value) : Value(value) {}
+
+  ~AtomicMaybeOwningPointer() {
+    // This can use relaxed memory order because the client has to ensure
+    // that all accesses are safely completed and their effects fully
+    // visible before destruction occurs anyway.
+    ::delete Value.load(std::memory_order_relaxed);
+  }
+};
+
 /// A concurrent map that is implemented using a binary tree. It supports
 /// concurrent insertions but does not support removals or rebalancing of
 /// the tree.
@@ -140,7 +165,8 @@ template <class ElemTy> struct ConcurrentList {
 ///   /// where KeyTy is the type of the first argument to getOrInsert and
 ///   /// ArgTys is the type of the remaining arguments.
 ///   static size_t getExtraAllocationSize(KeyTy key, ArgTys...)
-template <class EntryTy> class ConcurrentMap {
+template <class EntryTy, bool ProvideDestructor = true>
+class ConcurrentMap {
   struct Node {
     std::atomic<Node*> Left;
     std::atomic<Node*> Right;
@@ -182,7 +208,7 @@ template <class EntryTy> class ConcurrentMap {
   };
 
   /// The root of the tree.
-  std::atomic<Node*> Root;
+  AtomicMaybeOwningPointer<Node, ProvideDestructor> Root;
 
   /// This member stores the address of the last node that was found by the
   /// search procedure. We cache the last search to accelerate code that
@@ -195,13 +221,14 @@ template <class EntryTy> class ConcurrentMap {
   ConcurrentMap(const ConcurrentMap &) = delete;
   ConcurrentMap &operator=(const ConcurrentMap &) = delete;
 
-  ~ConcurrentMap() {
-    ::delete Root.load(std::memory_order_relaxed);
-  }
+  // ConcurrentMap<T, false> must have a trivial destructor.
+  ~ConcurrentMap() = default;
+
+public:
 
 #ifndef NDEBUG
   void dump() const {
-    auto R = Root.load(std::memory_order_acquire);
+    auto R = Root.Value.load(std::memory_order_acquire);
     printf("digraph g {\n"
            "graph [ rankdir = \"TB\"];\n"
            "node  [ fontsize = \"16\" ];\n"
@@ -225,7 +252,7 @@ template <class EntryTy> class ConcurrentMap {
     }
 
     // Search the tree, starting from the root.
-    Node *node = Root.load(std::memory_order_acquire);
+    Node *node = Root.Value.load(std::memory_order_acquire);
     while (node) {
       int comparisonResult = node->Payload.compareWithKey(key);
       if (comparisonResult == 0) {
@@ -258,7 +285,7 @@ template <class EntryTy> class ConcurrentMap {
     Node *newNode = nullptr;
 
     // Start from the root.
-    auto edge = &Root;
+    auto edge = &Root.Value;
 
     while (true) {
       // Load the edge.
@@ -313,4 +340,6 @@ template <class EntryTy> class ConcurrentMap {
   }
 };
 
+} // end namespace swift
+
 #endif // SWIFT_RUNTIME_CONCURRENTUTILS_H

stdlib/public/runtime/Demangle.cpp

@@ -209,10 +209,35 @@ Demangle::NodePointer swift::_swift_buildDemanglingForMetadata(const Metadata *t
   }
   case MetadataKind::Tuple: {
     auto tuple = static_cast<const TupleTypeMetadata *>(type);
+    const char *labels = tuple->Labels;
     auto tupleNode = NodeFactory::create(Node::Kind::NonVariadicTuple);
     for (unsigned i = 0, e = tuple->NumElements; i < e; ++i) {
-      auto elt = _swift_buildDemanglingForMetadata(tuple->getElement(i).Type);
-      tupleNode->addChild(elt);
+      auto elt = NodeFactory::create(Node::Kind::TupleElement);
+
+      // Add a label child if applicable:
+      if (labels) {
+        // Look for the next space in the labels string.
+        if (const char *space = strchr(labels, ' ')) {
+          // If there is one, and the label isn't empty, add a label child.
+          if (labels != space) {
+            auto eltName =
+              NodeFactory::create(Node::Kind::TupleElementName,
+                                  std::string(labels, space));
+            elt->addChild(std::move(eltName));
+          }
+
+          // Skip past the space.
+          labels = space + 1;
+        }
+      }
+
+      // Add the element type child.
+      auto eltType =
+        _swift_buildDemanglingForMetadata(tuple->getElement(i).Type);
+      elt->addChild(std::move(eltType));
+
+      // Add the completed element to the tuple.
+      tupleNode->addChild(std::move(elt));
     }
     return tupleNode;
   }