[llvm] ec129c2 - [YAML][NFC] Use BumpPtrAllocator instead of unique_ptrs

Amir Ayupov via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 9 16:42:31 PDT 2023


Author: Amir Ayupov
Date: 2023-08-09T16:42:25-07:00
New Revision: ec129c28af58d8200538184cab0ec659c437095d

URL: https://github.com/llvm/llvm-project/commit/ec129c28af58d8200538184cab0ec659c437095d
DIFF: https://github.com/llvm/llvm-project/commit/ec129c28af58d8200538184cab0ec659c437095d.diff

LOG: [YAML][NFC] Use BumpPtrAllocator instead of unique_ptrs

Avoid both memory leaks and expensive dynamic allocations by using
SpecificBumpPtrAllocator for HNode types. It's expected they're not deallocated
until the `Input` class is destroyed, so deallocating all at once works well in
this case.

Reduces YAML profile pre-processing time in BOLT from
>   11.2067 (  3.2%)   1.6487 (  7.3%)  12.8554 (  3.5%)  12.8635 (  5.6%)  pre-process profile data
to
>   10.6613 (  3.1%)   1.6489 (  6.7%)  12.3102 (  3.3%)  12.3134 (  5.3%)  pre-process profile data

Reviewed By: dblaikie

Differential Revision: https://reviews.llvm.org/D155006

Added: 
    

Modified: 
    llvm/include/llvm/Support/YAMLTraits.h
    llvm/lib/Support/YAMLTraits.cpp

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/Support/YAMLTraits.h b/llvm/include/llvm/Support/YAMLTraits.h
index 3ed29821fa8f03..b351ec629cbe58 100644
--- a/llvm/include/llvm/Support/YAMLTraits.h
+++ b/llvm/include/llvm/Support/YAMLTraits.h
@@ -1464,11 +1464,8 @@ class Input : public IO {
   bool canElideEmptySequence() override;
 
   class HNode {
-    virtual void anchor();
-
   public:
-    HNode(Node *n) : _node(n) { }
-    virtual ~HNode() = default;
+    HNode(Node *n) : _node(n) {}
 
     static bool classof(const HNode *) { return true; }
 
@@ -1476,8 +1473,6 @@ class Input : public IO {
   };
 
   class EmptyHNode : public HNode {
-    void anchor() override;
-
   public:
     EmptyHNode(Node *n) : HNode(n) { }
 
@@ -1487,8 +1482,6 @@ class Input : public IO {
   };
 
   class ScalarHNode : public HNode {
-    void anchor() override;
-
   public:
     ScalarHNode(Node *n, StringRef s) : HNode(n), _value(s) { }
 
@@ -1506,8 +1499,6 @@ class Input : public IO {
   };
 
   class MapHNode : public HNode {
-    void anchor() override;
-
   public:
     MapHNode(Node *n) : HNode(n) { }
 
@@ -1517,16 +1508,13 @@ class Input : public IO {
 
     static bool classof(const MapHNode *) { return true; }
 
-    using NameToNodeAndLoc =
-        StringMap<std::pair<std::unique_ptr<HNode>, SMRange>>;
+    using NameToNodeAndLoc = StringMap<std::pair<HNode *, SMRange>>;
 
     NameToNodeAndLoc Mapping;
     SmallVector<std::string, 6> ValidKeys;
   };
 
   class SequenceHNode : public HNode {
-    void anchor() override;
-
   public:
     SequenceHNode(Node *n) : HNode(n) { }
 
@@ -1536,10 +1524,10 @@ class Input : public IO {
 
     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);
   void setError(const SMRange &Range, const Twine &message);
@@ -1548,6 +1536,9 @@ class Input : public IO {
   void reportWarning(Node *hnode, const Twine &message);
   void reportWarning(const SMRange &Range, const Twine &message);
 
+  /// Release memory used by HNodes.
+  void releaseHNodeBuffers();
+
 public:
   // These are only used by operator>>. They could be private
   // if those templated things could be made friends.
@@ -1562,9 +1553,13 @@ class Input : public IO {
 private:
   SourceMgr                           SrcMgr; // must be before Strm
   std::unique_ptr<llvm::yaml::Stream> Strm;
-  std::unique_ptr<HNode>              TopNode;
+  HNode *TopNode = nullptr;
   std::error_code                     EC;
   BumpPtrAllocator                    StringAllocator;
+  SpecificBumpPtrAllocator<EmptyHNode> EmptyHNodeAllocator;
+  SpecificBumpPtrAllocator<ScalarHNode> ScalarHNodeAllocator;
+  SpecificBumpPtrAllocator<MapHNode> MapHNodeAllocator;
+  SpecificBumpPtrAllocator<SequenceHNode> SequenceHNodeAllocator;
   document_iterator                   DocIterator;
   llvm::BitVector                     BitValuesUsed;
   HNode *CurrentNode = nullptr;

diff  --git a/llvm/lib/Support/YAMLTraits.cpp b/llvm/lib/Support/YAMLTraits.cpp
index f21b7a0ca699ea..9325a09faaea02 100644
--- a/llvm/lib/Support/YAMLTraits.cpp
+++ b/llvm/lib/Support/YAMLTraits.cpp
@@ -75,13 +75,6 @@ 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() const {
   return false;
 }
@@ -99,8 +92,9 @@ bool Input::setCurrentDocument() {
       ++DocIterator;
       return setCurrentDocument();
     }
+    releaseHNodeBuffers();
     TopNode = createHNodes(N);
-    CurrentNode = TopNode.get();
+    CurrentNode = TopNode;
     return true;
   }
   return false;
@@ -174,7 +168,7 @@ bool Input::preflightKey(const char *Key, bool Required, bool, bool &UseDefault,
     return false;
   }
   MN->ValidKeys.push_back(Key);
-  HNode *Value = MN->Mapping[Key].first.get();
+  HNode *Value = MN->Mapping[Key].first;
   if (!Value) {
     if (Required)
       setError(CurrentNode, Twine("missing required key '") + Key + "'");
@@ -237,7 +231,7 @@ bool Input::preflightElement(unsigned Index, void *&SaveInfo) {
     return false;
   if (SequenceHNode *SQ = dyn_cast<SequenceHNode>(CurrentNode)) {
     SaveInfo = CurrentNode;
-    CurrentNode = SQ->Entries[Index].get();
+    CurrentNode = SQ->Entries[Index];
     return true;
   }
   return false;
@@ -254,7 +248,7 @@ bool Input::preflightFlowElement(unsigned index, void *&SaveInfo) {
     return false;
   if (SequenceHNode *SQ = dyn_cast<SequenceHNode>(CurrentNode)) {
     SaveInfo = CurrentNode;
-    CurrentNode = SQ->Entries[index].get();
+    CurrentNode = SQ->Entries[index];
     return true;
   }
   return false;
@@ -313,7 +307,7 @@ bool Input::bitSetMatch(const char *Str, bool) {
   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;
@@ -336,7 +330,7 @@ void Input::endBitSetScalar() {
     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;
       }
     }
@@ -395,7 +389,14 @@ void Input::reportWarning(const SMRange &range, const Twine &message) {
   Strm->printError(range, message, SourceMgr::DK_Warning);
 }
 
-std::unique_ptr<Input::HNode> Input::createHNodes(Node *N) {
+void Input::releaseHNodeBuffers() {
+  EmptyHNodeAllocator.DestroyAll();
+  ScalarHNodeAllocator.DestroyAll();
+  SequenceHNodeAllocator.DestroyAll();
+  MapHNodeAllocator.DestroyAll();
+}
+
+Input::HNode *Input::createHNodes(Node *N) {
   SmallString<128> StringStorage;
   switch (N->getType()) {
   case Node::NK_Scalar: {
@@ -405,27 +406,27 @@ std::unique_ptr<Input::HNode> Input::createHNodes(Node *N) {
       // Copy string to permanent storage
       KeyStr = StringStorage.str().copy(StringAllocator);
     }
-    return std::make_unique<ScalarHNode>(N, KeyStr);
+    return new (ScalarHNodeAllocator.Allocate()) ScalarHNode(N, KeyStr);
   }
   case Node::NK_BlockScalar: {
     BlockScalarNode *BSN = dyn_cast<BlockScalarNode>(N);
     StringRef ValueCopy = BSN->getValue().copy(StringAllocator);
-    return std::make_unique<ScalarHNode>(N, ValueCopy);
+    return new (ScalarHNodeAllocator.Allocate()) ScalarHNode(N, ValueCopy);
   }
   case Node::NK_Sequence: {
     SequenceNode *SQ = dyn_cast<SequenceNode>(N);
-    auto SQHNode = std::make_unique<SequenceHNode>(N);
+    auto SQHNode = new (SequenceHNodeAllocator.Allocate()) SequenceHNode(N);
     for (Node &SN : *SQ) {
       auto Entry = createHNodes(&SN);
       if (EC)
         break;
-      SQHNode->Entries.push_back(std::move(Entry));
+      SQHNode->Entries.push_back(Entry);
     }
-    return std::move(SQHNode);
+    return SQHNode;
   }
   case Node::NK_Mapping: {
     MappingNode *Map = dyn_cast<MappingNode>(N);
-    auto mapHNode = std::make_unique<MapHNode>(N);
+    auto mapHNode = new (MapHNodeAllocator.Allocate()) MapHNode(N);
     for (KeyValueNode &KVN : *Map) {
       Node *KeyNode = KVN.getKey();
       ScalarNode *Key = dyn_cast_or_null<ScalarNode>(KeyNode);
@@ -457,7 +458,7 @@ std::unique_ptr<Input::HNode> Input::createHNodes(Node *N) {
     return std::move(mapHNode);
   }
   case Node::NK_Null:
-    return std::make_unique<EmptyHNode>(N);
+    return new (EmptyHNodeAllocator.Allocate()) EmptyHNode(N);
   default:
     setError(N, "unknown node kind");
     return nullptr;


        


More information about the llvm-commits mailing list