[YAML] Use {Specific}BumpPtrAllocators for HNodes.

[ributzka]

Summary

The YAML parser already uses a BumpPtrAllocator for all its Nodes, but inside
YAMLTraits we still use make_unique for HNodes. This results in lot of malloc
traffic when parsing large or many YAML files.

Most HNode sub-classes can use a simple BumpPtrAllocator, but there are two
sub-classes that need SpecificBumpPtrAllocators to call the proper destructor.

[zturner]

llvm/lib/Support/YAMLTraits.cpp

357

This is a little confusing. Why are you calling Allocate<ScalarNode>() but then actually constructing a ScalarHNode instead of a ScalarNode?

[ributzka]

Updating D35908: [YAML] Use {Specific}BumpPtrAllocators for HNodes.

Fix a typo in the allocator argument. ScalarNode should have been ScalarHNode.

[ributzka]

llvm/lib/Support/YAMLTraits.cpp

357

Oops, that is a typo. Thanks for catching this.

llvm/include/llvm/Support/YAMLTraits.h

private:
   bool bitSetMatch(const char *, bool ) override;
   void endBitSetScalar() override;
   void scalarString(StringRef &, bool) override;
   void blockScalarString(StringRef &) override;
   void setError(const Twine &message) override;
   bool canElideEmptySequence() override;
 
   class HNode {
-    virtual void anchor();
-
   public:
     HNode(Node *n) : _node(n) { }
-    virtual ~HNode() = default;
 
     static bool classof(const HNode *) { return true; }
 
     Node *_node;
   };
 
   class EmptyHNode : public HNode {
-    void anchor() override;
-
   public:
     EmptyHNode(Node *n) : HNode(n) { }
 
     static bool classof(const HNode *n) { return NullNode::classof(n->_node); }
 
     static bool classof(const EmptyHNode *) { return true; }
   };
 
   class ScalarHNode : public HNode {
-    void anchor() override;
-
   public:
     ScalarHNode(Node *n, StringRef s) : HNode(n), _value(s) { }
 
     StringRef value() const { return _value; }
 
     static bool classof(const HNode *n) {
       return ScalarNode::classof(n->_node) ||
              BlockScalarNode::classof(n->_node);
     }
 
     static bool classof(const ScalarHNode *) { return true; }
 
   protected:
     StringRef _value;
   };
 
   class MapHNode : public HNode {
-    void anchor() override;
-
   public:
     MapHNode(Node *n) : HNode(n) { }
 
     static bool classof(const HNode *n) {
       return MappingNode::classof(n->_node);
     }
 
     static bool classof(const MapHNode *) { return true; }
 
-    using NameToNode = StringMap<std::unique_ptr<HNode>>;
+    using NameToNode = StringMap<HNode *>;
 
     NameToNode Mapping;
     SmallVector<std::string, 6> ValidKeys;
   };
 
   class SequenceHNode : public HNode {
-    void anchor() override;
-
   public:
     SequenceHNode(Node *n) : HNode(n) { }
 
     static bool classof(const HNode *n) {
       return SequenceNode::classof(n->_node);
     }
 
     static bool classof(const SequenceHNode *) { return true; }
 
-    std::vector<std::unique_ptr<HNode>> Entries;
+    std::vector<HNode *> Entries;
   };
 
-  std::unique_ptr<Input::HNode> createHNodes(Node *node);
+  Input::HNode *createHNodes(Node *node);
   void setError(HNode *hnode, const Twine &message);
   void setError(Node *node, const Twine &message);
 
 public:
   // These are only used by operator>>. They could be private
   // if those templated things could be made friends.
   bool setCurrentDocument();
   bool nextDocument();
 
   /// Returns the current node that's being parsed by the YAML Parser.
   const Node *getCurrentNode() const;
 
 private:
   SourceMgr                           SrcMgr; // must be before Strm
   std::unique_ptr<llvm::yaml::Stream> Strm;
-  std::unique_ptr<HNode>              TopNode;
+  HNode *TopNode;
   std::error_code                     EC;
   BumpPtrAllocator                    StringAllocator;
+  BumpPtrAllocator                    HNodeAllocator;
+  SpecificBumpPtrAllocator<SequenceHNode> SequenceHNodeAllocator;
+  SpecificBumpPtrAllocator<MapHNode> MapHNodeAllocator;
   document_iterator                   DocIterator;
   std::vector<bool>                   BitValuesUsed;
   HNode *CurrentNode = nullptr;
   bool                                ScalarMatchFound;
 };
 
 ///
 /// The Output class is used to generate a yaml document from in-memory structs

llvm/lib/Support/YAMLTraits.cpp

if (DiagHandler)
     SrcMgr.setDiagHandler(DiagHandler, DiagHandlerCtxt);
   DocIterator = Strm->begin();
 }
 
 Input::~Input() = default;
 
 std::error_code Input::error() { return EC; }
 
-// Pin the vtables to this file.
-void Input::HNode::anchor() {}
-void Input::EmptyHNode::anchor() {}
-void Input::ScalarHNode::anchor() {}
-void Input::MapHNode::anchor() {}
-void Input::SequenceHNode::anchor() {}
-
 bool Input::outputting() {
   return false;
 }
 
 bool Input::setCurrentDocument() {
   if (DocIterator != Strm->end()) {
     Node *N = DocIterator->getRoot();
     if (!N) {
       assert(Strm->failed() && "Root is NULL iff parsing failed");
       EC = make_error_code(errc::invalid_argument);
       return false;
     }
 
     if (isa<NullNode>(N)) {
       // Empty files are allowed and ignored
       ++DocIterator;
       return setCurrentDocument();
     }
+    TopNode = CurrentNode = nullptr;
+    HNodeAllocator.Reset();
+    SequenceHNodeAllocator.DestroyAll();
+    MapHNodeAllocator.DestroyAll();
     TopNode = this->createHNodes(N);
-    CurrentNode = TopNode.get();
+    CurrentNode = TopNode;
     return true;
   }
   return false;
 }
 
 bool Input::nextDocument() {
   return ++DocIterator != Strm->end();
 }
bool Input::preflightKey(const char *Key, bool Required, bool, bool &UseDefault,
   }
 
   MapHNode *MN = dyn_cast<MapHNode>(CurrentNode);
   if (!MN) {
     setError(CurrentNode, "not a mapping");
     return false;
   }
   MN->ValidKeys.push_back(Key);
-  HNode *Value = MN->Mapping[Key].get();
+  HNode *Value = MN->Mapping[Key];
   if (!Value) {
     if (Required)
       setError(CurrentNode, Twine("missing required key '") + Key + "'");
     else
       UseDefault = true;
     return false;
   }
   SaveInfo = CurrentNode;
void Input::endMapping() {
   if (EC)
     return;
   // CurrentNode can be null if the document is empty.
   MapHNode *MN = dyn_cast_or_null<MapHNode>(CurrentNode);
   if (!MN)
     return;
   for (const auto &NN : MN->Mapping) {
     if (!is_contained(MN->ValidKeys, NN.first())) {
-      setError(NN.second.get(), Twine("unknown key '") + NN.first() + "'");
+      setError(NN.second, Twine("unknown key '") + NN.first() + "'");
       break;
     }
   }
 }
 
 void Input::beginFlowMapping() { beginMapping(); }
 
 void Input::endFlowMapping() { endMapping(); }

 void Input::endSequence() {
 }
 
 bool Input::preflightElement(unsigned Index, void *&SaveInfo) {
   if (EC)
     return false;
   if (SequenceHNode *SQ = dyn_cast<SequenceHNode>(CurrentNode)) {
     SaveInfo = CurrentNode;
-    CurrentNode = SQ->Entries[Index].get();
+    CurrentNode = SQ->Entries[Index];
     return true;
   }
   return false;
 }
 
 void Input::postflightElement(void *SaveInfo) {
   CurrentNode = reinterpret_cast<HNode *>(SaveInfo);
 }
 
 unsigned Input::beginFlowSequence() { return beginSequence(); }
 
 bool Input::preflightFlowElement(unsigned index, void *&SaveInfo) {
   if (EC)
     return false;
   if (SequenceHNode *SQ = dyn_cast<SequenceHNode>(CurrentNode)) {
     SaveInfo = CurrentNode;
-    CurrentNode = SQ->Entries[index].get();
+    CurrentNode = SQ->Entries[index];
     return true;
   }
   return false;
 }
 
 void Input::postflightFlowElement(void *SaveInfo) {
   CurrentNode = reinterpret_cast<HNode *>(SaveInfo);
 }

 }
 
 bool Input::bitSetMatch(const char *Str, bool) {
   if (EC)
     return false;
   if (SequenceHNode *SQ = dyn_cast<SequenceHNode>(CurrentNode)) {
     unsigned Index = 0;
     for (auto &N : SQ->Entries) {
-      if (ScalarHNode *SN = dyn_cast<ScalarHNode>(N.get())) {
+      if (ScalarHNode *SN = dyn_cast<ScalarHNode>(N)) {
         if (SN->value().equals(Str)) {
           BitValuesUsed[Index] = true;
           return true;
         }
       } else {
         setError(CurrentNode, "unexpected scalar in sequence of bit values");
       }
       ++Index;
     }
   } else {
     setError(CurrentNode, "expected sequence of bit values");
   }
   return false;
 }
 
 void Input::endBitSetScalar() {
   if (EC)
     return;
   if (SequenceHNode *SQ = dyn_cast<SequenceHNode>(CurrentNode)) {
     assert(BitValuesUsed.size() == SQ->Entries.size());
     for (unsigned i = 0; i < SQ->Entries.size(); ++i) {
       if (!BitValuesUsed[i]) {
-        setError(SQ->Entries[i].get(), "unknown bit value");
+        setError(SQ->Entries[i], "unknown bit value");
         return;
       }
     }
   }
 }
 
 void Input::scalarString(StringRef &S, bool) {
   if (ScalarHNode *SN = dyn_cast<ScalarHNode>(CurrentNode)) {
void Input::setError(HNode *hnode, const Twine &message) {
   this->setError(hnode->_node, message);
 }
 
 void Input::setError(Node *node, const Twine &message) {
   Strm->printError(node, message);
   EC = make_error_code(errc::invalid_argument);
 }
 
-std::unique_ptr<Input::HNode> Input::createHNodes(Node *N) {
+Input::HNode *Input::createHNodes(Node *N) {
   SmallString<128> StringStorage;
   if (ScalarNode *SN = dyn_cast<ScalarNode>(N)) {
     StringRef KeyStr = SN->getValue(StringStorage);
     if (!StringStorage.empty()) {
       // Copy string to permanent storage
       KeyStr = StringStorage.str().copy(StringAllocator);
     }
-    return llvm::make_unique<ScalarHNode>(N, KeyStr);
+    return new (HNodeAllocator.Allocate<ScalarHNode>()) ScalarHNode(N, KeyStr);
   } else if (BlockScalarNode *BSN = dyn_cast<BlockScalarNode>(N)) {
     StringRef ValueCopy = BSN->getValue().copy(StringAllocator);
-    return llvm::make_unique<ScalarHNode>(N, ValueCopy);
+    return new (HNodeAllocator.Allocate<ScalarHNode>())
+        ScalarHNode(N, ValueCopy);
   } else if (SequenceNode *SQ = dyn_cast<SequenceNode>(N)) {
-    auto SQHNode = llvm::make_unique<SequenceHNode>(N);
+    auto SQHNode = new (SequenceHNodeAllocator.Allocate()) SequenceHNode(N);
     for (Node &SN : *SQ) {
       auto Entry = this->createHNodes(&SN);
       if (EC)
         break;
-      SQHNode->Entries.push_back(std::move(Entry));
+      SQHNode->Entries.push_back(Entry);
     }
-    return std::move(SQHNode);
+    return SQHNode;
   } else if (MappingNode *Map = dyn_cast<MappingNode>(N)) {
-    auto mapHNode = llvm::make_unique<MapHNode>(N);
+    auto mapHNode = new (MapHNodeAllocator.Allocate()) MapHNode(N);
     for (KeyValueNode &KVN : *Map) {
       Node *KeyNode = KVN.getKey();
       ScalarNode *KeyScalar = dyn_cast<ScalarNode>(KeyNode);
       if (!KeyScalar) {
         setError(KeyNode, "Map key must be a scalar");
         break;
       }
       StringStorage.clear();
       StringRef KeyStr = KeyScalar->getValue(StringStorage);
       if (!StringStorage.empty()) {
         // Copy string to permanent storage
         KeyStr = StringStorage.str().copy(StringAllocator);
       }
       auto ValueHNode = this->createHNodes(KVN.getValue());
       if (EC)
         break;
-      mapHNode->Mapping[KeyStr] = std::move(ValueHNode);
+      mapHNode->Mapping[KeyStr] = ValueHNode;
     }
-    return std::move(mapHNode);
+    return mapHNode;
   } else if (isa<NullNode>(N)) {
-    return llvm::make_unique<EmptyHNode>(N);
+    return new (HNodeAllocator.Allocate<EmptyHNode>()) EmptyHNode(N);
   } else {
     setError(N, "unknown node kind");
     return nullptr;
   }
 }
 
 void Input::setError(const Twine &Message) {
   this->setError(CurrentNode, Message);