[Remarks] String tables should be move-only

Copying them is expensive. This allows the tables to be moved around at
lower cost, and allows a remarks::StringTable to be constructed from
a remarks::ParsedStringTable.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@366864 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/llvm/Remarks/RemarkParser.h b/include/llvm/Remarks/RemarkParser.h
index eb827d130fa4f89f95ed9f0a93331a8648dea75e..f53c04db8ea686c40a95b0023c543c10b07ef15f 100644 (file)
--- a/include/llvm/Remarks/RemarkParser.h
+++ b/include/llvm/Remarks/RemarkParser.h
@@ -23,9 +23,6 @@
 namespace llvm {
 namespace remarks {
 
-struct ParserImpl;
-struct ParsedStringTable;
-
 class EndOfFileError : public ErrorInfo<EndOfFileError> {
 public:
   static char ID;
@@ -60,19 +57,28 @@ struct Parser {
 struct ParsedStringTable {
   /// The buffer mapped from the section contents.
   StringRef Buffer;
-  /// Collection of offsets in the buffer for each string entry.
-  SmallVector<size_t, 8> Offsets;
+  /// This object has high changes to be std::move'd around, so don't use a
+  /// SmallVector for once.
+  std::vector<size_t> Offsets;
 
-  Expected<StringRef> operator[](size_t Index) const;
   ParsedStringTable(StringRef Buffer);
+  /// Disable copy.
+  ParsedStringTable(const ParsedStringTable &) = delete;
+  ParsedStringTable &operator=(const ParsedStringTable &) = delete;
+  /// Should be movable.
+  ParsedStringTable(ParsedStringTable &&) = default;
+  ParsedStringTable &operator=(ParsedStringTable &&) = default;
+
+  size_t size() const { return Offsets.size(); }
+  Expected<StringRef> operator[](size_t Index) const;
 };
 
 Expected<std::unique_ptr<Parser>> createRemarkParser(Format ParserFormat,
                                                      StringRef Buf);
 
-Expected<std::unique_ptr<Parser>>
-createRemarkParser(Format ParserFormat, StringRef Buf,
-                   const ParsedStringTable &StrTab);
+Expected<std::unique_ptr<Parser>> createRemarkParser(Format ParserFormat,
+                                                     StringRef Buf,
+                                                     ParsedStringTable StrTab);
 
 } // end namespace remarks
 } // end namespace llvm

diff --git a/include/llvm/Remarks/RemarkStringTable.h b/include/llvm/Remarks/RemarkStringTable.h
index f9b4fdbbfb8d570b61c11495ed22f7d60df55e84..be2596d42c886f0da4adc78466bf035f3341e933 100644 (file)
--- a/include/llvm/Remarks/RemarkStringTable.h
+++ b/include/llvm/Remarks/RemarkStringTable.h
@@ -18,7 +18,7 @@
 
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
-#include "llvm/Support/Allocator.h"
+#include "llvm/Remarks/RemarkParser.h"
 #include <vector>
 
 namespace llvm {
@@ -31,15 +31,24 @@ namespace remarks {
 /// This table can be for example serialized in a section to be consumed after
 /// the compilation.
 struct StringTable {
-  /// Allocator holding all the memory used by the map.
-  BumpPtrAllocator Allocator;
   /// The string table containing all the unique strings used in the output.
   /// It maps a string to an unique ID.
-  StringMap<unsigned, BumpPtrAllocator &> StrTab;
+  StringMap<unsigned, BumpPtrAllocator> StrTab;
   /// Total size of the string table when serialized.
   size_t SerializedSize = 0;
 
-  StringTable() : Allocator(), StrTab(Allocator) {}
+  StringTable() = default;
+
+  /// Disable copy.
+  StringTable(const StringTable &) = delete;
+  StringTable &operator=(const StringTable &) = delete;
+  /// Should be movable.
+  StringTable(StringTable &&) = default;
+  StringTable &operator=(StringTable &&) = default;
+
+  /// Construct a string table from a ParsedStringTable.
+  StringTable(const ParsedStringTable &Other);
+
   /// Add a string to the table. It returns an unique ID of the string.
   std::pair<unsigned, StringRef> add(StringRef Str);
   /// Serialize the string table to a stream. It is serialized as a little

diff --git a/lib/Remarks/RemarkParser.cpp b/lib/Remarks/RemarkParser.cpp
index 96a4b42e8133e61d0ba406d20aeb12d2a0dc75c5..ac46ae3f1e01d5b6c96f6d510cdaf764e49b98b2 100644 (file)
--- a/lib/Remarks/RemarkParser.cpp
+++ b/lib/Remarks/RemarkParser.cpp
@@ -64,14 +64,14 @@ llvm::remarks::createRemarkParser(Format ParserFormat, StringRef Buf) {
 
 Expected<std::unique_ptr<Parser>>
 llvm::remarks::createRemarkParser(Format ParserFormat, StringRef Buf,
-                                  const ParsedStringTable &StrTab) {
+                                  ParsedStringTable StrTab) {
   switch (ParserFormat) {
   case Format::YAML:
     return createStringError(std::make_error_code(std::errc::invalid_argument),
                              "The YAML format can't be used with a string "
                              "table. Use yaml-strtab instead.");
   case Format::YAMLStrTab:
-    return llvm::make_unique<YAMLStrTabRemarkParser>(Buf, StrTab);
+    return llvm::make_unique<YAMLStrTabRemarkParser>(Buf, std::move(StrTab));
   case Format::Unknown:
     return createStringError(std::make_error_code(std::errc::invalid_argument),
                              "Unknown remark parser format.");
@@ -84,10 +84,10 @@ struct CParser {
   Optional<std::string> Err;
 
   CParser(Format ParserFormat, StringRef Buf,
-          Optional<const ParsedStringTable *> StrTab = None)
-      : TheParser(cantFail(StrTab
-                               ? createRemarkParser(ParserFormat, Buf, **StrTab)
-                               : createRemarkParser(ParserFormat, Buf))) {}
+          Optional<ParsedStringTable> StrTab = None)
+      : TheParser(cantFail(
+            StrTab ? createRemarkParser(ParserFormat, Buf, std::move(*StrTab))
+                   : createRemarkParser(ParserFormat, Buf))) {}
 
   void handleError(Error E) { Err.emplace(toString(std::move(E))); }
   bool hasError() const { return Err.hasValue(); }

diff --git a/lib/Remarks/RemarkStringTable.cpp b/lib/Remarks/RemarkStringTable.cpp
index 90a90e5757faaa8caf0ca502a42a5b6374b6c98b..17aa3944924347ea2dcef24e310aae6cc6f534f2 100644 (file)
--- a/lib/Remarks/RemarkStringTable.cpp
+++ b/lib/Remarks/RemarkStringTable.cpp
@@ -11,6 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Remarks/RemarkStringTable.h"
+#include "llvm/Remarks/RemarkParser.h"
 #include "llvm/Support/EndianStream.h"
 #include "llvm/Support/Error.h"
 #include <vector>
@@ -18,6 +19,14 @@
 using namespace llvm;
 using namespace llvm::remarks;
 
+StringTable::StringTable(const ParsedStringTable &Other) : StrTab() {
+  for (unsigned i = 0, e = Other.size(); i < e; ++i)
+    if (Expected<StringRef> MaybeStr = Other[i])
+      add(*MaybeStr);
+    else
+      llvm_unreachable("Unexpected error while building remarks string table.");
+}
+
 std::pair<unsigned, StringRef> StringTable::add(StringRef Str) {
   size_t NextID = StrTab.size();
   auto KV = StrTab.insert({Str, NextID});

diff --git a/lib/Remarks/YAMLRemarkParser.cpp b/lib/Remarks/YAMLRemarkParser.cpp
index 3cbb4932e0ab89aba8f77f1127facb21383ce798..fcb4cce099b868bba77089ccceed0549fbcf03bc 100644 (file)
--- a/lib/Remarks/YAMLRemarkParser.cpp
+++ b/lib/Remarks/YAMLRemarkParser.cpp
@@ -58,8 +58,8 @@ YAMLRemarkParser::YAMLRemarkParser(StringRef Buf)
     : YAMLRemarkParser(Buf, None) {}
 
 YAMLRemarkParser::YAMLRemarkParser(StringRef Buf,
-                                   Optional<const ParsedStringTable *> StrTab)
-    : Parser{Format::YAML}, StrTab(StrTab), LastErrorMessage(),
+                                   Optional<ParsedStringTable> StrTab)
+    : Parser{Format::YAML}, StrTab(std::move(StrTab)), LastErrorMessage(),
       SM(setupSM(LastErrorMessage)), Stream(Buf, SM), YAMLIt(Stream.begin()) {}
 
 Error YAMLRemarkParser::error(StringRef Message, yaml::Node &Node) {
@@ -326,7 +326,7 @@ Expected<StringRef> YAMLStrTabRemarkParser::parseStr(yaml::KeyValueNode &Node) {
   else
     return MaybeStrID.takeError();
 
-  if (Expected<StringRef> Str = (**StrTab)[StrID])
+  if (Expected<StringRef> Str = (*StrTab)[StrID])
     Result = *Str;
   else
     return Str.takeError();

diff --git a/lib/Remarks/YAMLRemarkParser.h b/lib/Remarks/YAMLRemarkParser.h
index 82a86fa780a60281f43e0b45a66d0375a27ca8fc..da0e9de0f4d1daab418ed8ba813dbca302938d82 100644 (file)
--- a/lib/Remarks/YAMLRemarkParser.h
+++ b/lib/Remarks/YAMLRemarkParser.h
@@ -48,7 +48,7 @@ private:
 /// Regular YAML to Remark parser.
 struct YAMLRemarkParser : public Parser {
   /// The string table used for parsing strings.
-  Optional<const ParsedStringTable *> StrTab;
+  Optional<ParsedStringTable> StrTab;
   /// Last error message that can come from the YAML parser diagnostics.
   /// We need this for catching errors in the constructor.
   std::string LastErrorMessage;
@@ -68,7 +68,7 @@ struct YAMLRemarkParser : public Parser {
   }
 
 protected:
-  YAMLRemarkParser(StringRef Buf, Optional<const ParsedStringTable *> StrTab);
+  YAMLRemarkParser(StringRef Buf, Optional<ParsedStringTable> StrTab);
   /// Create a YAMLParseError error from an existing error generated by the YAML
   /// parser.
   /// If there is no error, this returns Success.
@@ -93,8 +93,8 @@ protected:
 
 /// YAML with a string table to Remark parser.
 struct YAMLStrTabRemarkParser : public YAMLRemarkParser {
-  YAMLStrTabRemarkParser(StringRef Buf, const ParsedStringTable &StrTab)
-      : YAMLRemarkParser(Buf, &StrTab) {}
+  YAMLStrTabRemarkParser(StringRef Buf, ParsedStringTable StrTab)
+      : YAMLRemarkParser(Buf, std::move(StrTab)) {}
 
   static bool classof(const Parser *P) {
     return P->ParserFormat == Format::YAMLStrTab;

diff --git a/unittests/Remarks/YAMLRemarksParsingTest.cpp b/unittests/Remarks/YAMLRemarksParsingTest.cpp
index 84d1371457ba70821242e8150429a7e52bb3c6a3..baa481409f9e3122cb78a91a2f3d11a63a1c550d 100644 (file)
--- a/unittests/Remarks/YAMLRemarksParsingTest.cpp
+++ b/unittests/Remarks/YAMLRemarksParsingTest.cpp
@@ -526,7 +526,8 @@ TEST(YAMLRemarks, ContentsStrTab) {
 
   remarks::ParsedStringTable StrTab(StrTabBuf);
   Expected<std::unique_ptr<remarks::Parser>> MaybeParser =
-      remarks::createRemarkParser(remarks::Format::YAMLStrTab, Buf, StrTab);
+      remarks::createRemarkParser(remarks::Format::YAMLStrTab, Buf,
+                                  std::move(StrTab));
   EXPECT_FALSE(errorToBool(MaybeParser.takeError()));
   EXPECT_TRUE(*MaybeParser != nullptr);
 
@@ -601,7 +602,8 @@ TEST(YAMLRemarks, ParsingBadStringTableIndex) {
 
   remarks::ParsedStringTable StrTab(StrTabBuf);
   Expected<std::unique_ptr<remarks::Parser>> MaybeParser =
-      remarks::createRemarkParser(remarks::Format::YAMLStrTab, Buf, StrTab);
+      remarks::createRemarkParser(remarks::Format::YAMLStrTab, Buf,
+                                  std::move(StrTab));
   EXPECT_FALSE(errorToBool(MaybeParser.takeError()));
   EXPECT_TRUE(*MaybeParser != nullptr);