[PATCH] D60227: [Remarks] Add string deduplication using a string table

[Jonas Devlieghere via Phabricator via llvm-commits]

JDevlieghere added inline comments.


================
Comment at: llvm/include/llvm/Remarks/RemarkStringTable.h:40
+  /// Total size of the string table when serialized.
+  size_t SerializedSize = 0;
+
----------------
Can we not use the size of the StringMap?


================
Comment at: llvm/lib/IR/DiagnosticInfo.cpp:428
+      io.mapOptional("DebugLoc", DL);
+    io.mapRequired("Function", FunctionID);
+    io.mapOptional("Hotness", OptDiag->Hotness);
----------------
Since the keys are the same and only the values are different, I'd extract this in a helper function. 


================
Comment at: llvm/lib/Remarks/RemarkParser.cpp:85
+
+  size_t NextOffset = Offsets[Index + 1];
+  return StringRef(Buffer.data() + Offset, NextOffset - Offset - 1);
----------------
Not sure if this would actually simplify things, but you could do
```
size_t NextOffset = (Index == Offsets.size() - 1) ? Buffer.size() : Offsets[Index + 1];
```
to not repeat the computation in the StringRef ctor.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60227/new/

https://reviews.llvm.org/D60227