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

Zachary Turner via llvm-commits llvm-commits at lists.llvm.org
Thu May 26 00:00:58 PDT 2016


zturner added a comment.

In addition to the above comments, I went ahead and fixed `VarStreamArray` to work, and I am actually using it to iterate over the `ModInfo`s that we were previously using a custom iterator for.  In order to make this work, I had to introduce a new `StreamObject` class.  In the future we may even be able to simplify the code of `FixedStreamArray<T>` by making it internally use `StreamObject<T>` to handle some of the logic.

Since this class now works correctly for the ModuleInfo array, I expect it will be simple to extend it to the symbol and type streams, but I want to do that in a followup patch.


================
Comment at: include/llvm/DebugInfo/CodeView/StreamArray.h:97
@@ +96,3 @@
+private:
+  const VarStreamArray *Array;
+  uint32_t ThisLen;
----------------
If T has a non-trivial constructor, this class won't work anyway because we are reinterpret-casting a raw byte stream to the object type.  So, in fact, this class *should* fail to compile for classes with non-trivial constructors.

================
Comment at: lib/DebugInfo/CodeView/StreamArray.cpp:20
@@ +19,3 @@
+
+  const CacheItem &Item = RecordOffsets[Index];
+  ArrayRef<uint8_t> Data;
----------------
ruiu wrote:
> So RecordOffsets maps each byte position to each byte position, correct? I wonder if it's going to be too much. If you access sequentially from 0 to 100MB using operator[], is it going to insert 100 million items to the hash?
Yea, maybe so.  I think at some point we may need something like this, but on the other hand maybe we can use the integrated hash table for those.  For now I've removed this and we support forward only iteration.


http://reviews.llvm.org/D20654





More information about the llvm-commits mailing list