[llvm] r366864 - [Remarks] String tables should be move-only
Francis Visoiu Mistrih via llvm-commits
llvm-commits at lists.llvm.org
Tue Jul 23 15:50:08 PDT 2019
Author: thegameg
Date: Tue Jul 23 15:50:08 2019
New Revision: 366864
URL: http://llvm.org/viewvc/llvm-project?rev=366864&view=rev
Log:
[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.
Modified:
llvm/trunk/include/llvm/Remarks/RemarkParser.h
llvm/trunk/include/llvm/Remarks/RemarkStringTable.h
llvm/trunk/lib/Remarks/RemarkParser.cpp
llvm/trunk/lib/Remarks/RemarkStringTable.cpp
llvm/trunk/lib/Remarks/YAMLRemarkParser.cpp
llvm/trunk/lib/Remarks/YAMLRemarkParser.h
llvm/trunk/unittests/Remarks/YAMLRemarksParsingTest.cpp
Modified: llvm/trunk/include/llvm/Remarks/RemarkParser.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Remarks/RemarkParser.h?rev=366864&r1=366863&r2=366864&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Remarks/RemarkParser.h (original)
+++ llvm/trunk/include/llvm/Remarks/RemarkParser.h Tue Jul 23 15:50:08 2019
@@ -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
Modified: llvm/trunk/include/llvm/Remarks/RemarkStringTable.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/Remarks/RemarkStringTable.h?rev=366864&r1=366863&r2=366864&view=diff
==============================================================================
--- llvm/trunk/include/llvm/Remarks/RemarkStringTable.h (original)
+++ llvm/trunk/include/llvm/Remarks/RemarkStringTable.h Tue Jul 23 15:50:08 2019
@@ -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
Modified: llvm/trunk/lib/Remarks/RemarkParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Remarks/RemarkParser.cpp?rev=366864&r1=366863&r2=366864&view=diff
==============================================================================
--- llvm/trunk/lib/Remarks/RemarkParser.cpp (original)
+++ llvm/trunk/lib/Remarks/RemarkParser.cpp Tue Jul 23 15:50:08 2019
@@ -64,14 +64,14 @@ llvm::remarks::createRemarkParser(Format
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(); }
Modified: llvm/trunk/lib/Remarks/RemarkStringTable.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Remarks/RemarkStringTable.cpp?rev=366864&r1=366863&r2=366864&view=diff
==============================================================================
--- llvm/trunk/lib/Remarks/RemarkStringTable.cpp (original)
+++ llvm/trunk/lib/Remarks/RemarkStringTable.cpp Tue Jul 23 15:50:08 2019
@@ -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});
Modified: llvm/trunk/lib/Remarks/YAMLRemarkParser.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Remarks/YAMLRemarkParser.cpp?rev=366864&r1=366863&r2=366864&view=diff
==============================================================================
--- llvm/trunk/lib/Remarks/YAMLRemarkParser.cpp (original)
+++ llvm/trunk/lib/Remarks/YAMLRemarkParser.cpp Tue Jul 23 15:50:08 2019
@@ -58,8 +58,8 @@ YAMLRemarkParser::YAMLRemarkParser(Strin
: 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> YAMLStrTabRemarkPars
else
return MaybeStrID.takeError();
- if (Expected<StringRef> Str = (**StrTab)[StrID])
+ if (Expected<StringRef> Str = (*StrTab)[StrID])
Result = *Str;
else
return Str.takeError();
Modified: llvm/trunk/lib/Remarks/YAMLRemarkParser.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Remarks/YAMLRemarkParser.h?rev=366864&r1=366863&r2=366864&view=diff
==============================================================================
--- llvm/trunk/lib/Remarks/YAMLRemarkParser.h (original)
+++ llvm/trunk/lib/Remarks/YAMLRemarkParser.h Tue Jul 23 15:50:08 2019
@@ -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;
Modified: llvm/trunk/unittests/Remarks/YAMLRemarksParsingTest.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/unittests/Remarks/YAMLRemarksParsingTest.cpp?rev=366864&r1=366863&r2=366864&view=diff
==============================================================================
--- llvm/trunk/unittests/Remarks/YAMLRemarksParsingTest.cpp (original)
+++ llvm/trunk/unittests/Remarks/YAMLRemarksParsingTest.cpp Tue Jul 23 15:50:08 2019
@@ -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, ParsingBadStringTableI
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);
More information about the llvm-commits
mailing list