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

[Zachary Turner via llvm-commits]

zturner added inline comments.

================
Comment at: include/llvm/DebugInfo/CodeView/StreamObject.h:26
@@ +25,3 @@
+      consumeError(std::move(EC));
+      TempValue.reset(new uint8_t[sizeof(T)]);
+      MutableArrayRef<uint8_t> Buf(TempValue.get(), sizeof(T));
----------------
ruiu wrote:
> sizeof(T) is a compile-time constant. Do you need to allocate it dynamically?
I did this because I didn't want `sizeof(StreamObject)` to be >= `sizeof(T)`.  It's probably minor, but in any case I should be able to fix this after switching to the pooled allocator.

================
Comment at: lib/DebugInfo/CodeView/StreamReader.cpp:57-60
@@ +56,6 @@
+Error StreamReader::readZeroString(StreamString &Dest) {
+  std::string S;
+  uint32_t OldOff = getOffset();
+  if (auto EC = readZeroString(S))
+    return EC;
+  uint32_t NewOff = getOffset();
----------------
ruiu wrote:
> This seems to be copying the string contents to the local std::string object whether the source string is contiguous in memory or not. Am I missing something?
This was kind of a weird function to write.  Yes, it copies the source string to a temporary `std::string` object.  It does this in order to figure out how long the string is.  The logic of scanning one by one for a `\0` is already handled in the ovther overload of this function and I didn't want to reproduce it.  So after it returns the string in a temporary, it can calculate the length `N`.  Then, it tries to get `N` bytes contiguously (see the call to `getArrayRef` below.  If it works, yay, make a `StreamString` with a reference to those `N` bytes.  If it didn't work, make a `StreamString` using the temporary `std::string` (which will make a copy of the temporary variable into its own internal `std::string`, and then return a reference to that).

In any case, I'm re-working all this stuff to use the pooled allocator, because since using the temporary storage is so rare, it doesn't make sense to have the overhead of storing a whole `std::string` every time.


http://reviews.llvm.org/D20654