[PATCH] D20654: [pdb] Try super hard to conserve memory in llvm-pdbdump

[Rui Ueyama via llvm-commits]

ruiu added a comment.

Generally looking good. I think I now fully understand how it works.


================
Comment at: include/llvm/DebugInfo/CodeView/StreamArray.h:127-129
@@ +126,5 @@
+    if (auto EC = Stream.readBytes(Off, sizeof(T), Data)) {
+      // This should never happen since we asserted that the stream length was
+      // an exact multiple of the element size.
+      consumeError(std::move(EC));
+    }
----------------
If this should never happen, you want to make it an assertion instead of swallowing errors.

================
Comment at: lib/DebugInfo/CodeView/StreamReader.cpp:76-79
@@ -48,1 +75,6 @@
+Error StreamReader::readFixedString(StringRef &Dest, uint32_t Length) {
+  ArrayRef<uint8_t> Bytes;
+  if (auto EC = readBytes(Length, Bytes))
     return EC;
+  Dest = StringRef(reinterpret_cast<const char *>(Bytes.begin()), Bytes.size());
+  return Error::success();
----------------
This looks much better now.

================
Comment at: lib/DebugInfo/PDB/Raw/MappedBlockStream.cpp:39-40
@@ +38,4 @@
+  uint32_t BytesInFirstBlock = Pdb.getBlockSize() - OffsetInBlock;
+  // If the entire request can be satisfied from one block, return an
+  // ArrayRef directly into the block.
+  if (BytesInFirstBlock >= Size) {
----------------
How is it likely that a object we want to fetch spans two blocks but the two blocks are contiguous in the file? If it is likely, you might want to optimize for that case. (In that case you don't need to copy contents.)


http://reviews.llvm.org/D20654