[PATCH] D28113: Use lazy-loading of Metadata in MetadataLoader when importing is enabled (NFC)

[Mehdi AMINI via Phabricator via llvm-commits]

mehdi_amini marked 6 inline comments as done.
mehdi_amini added inline comments.


================
Comment at: llvm/lib/Bitcode/Reader/MetadataLoader.cpp:625
+        // value expliticitly.
+        IndexCursor.JumpToBit(CurrentPos);
+        Record.clear();
----------------
tejohnson wrote:
> Should NumMDRecordLoaded be incremented in this case too? Maybe it would be better to CSE the increment of that down into a single place at the end of the BitstreamEntry::Record block (i.e. after the switch on Code, so after any case that doesn't cause an early return)?
Good point!


================
Comment at: llvm/lib/Bitcode/Reader/MetadataLoader.cpp:1501
+
+Error MetadataLoader::MetadataLoaderImpl::indexMetadataStrings(
+    ArrayRef<uint64_t> Record, StringRef Blob) {
----------------
tejohnson wrote:
> This is largely the same as the code in parseMetadataStrings. Could it be merged with a callback to do the right thing with each resulting StringRef?
> 
> Also, since we are essentially always parsing all the strings, what is the cost of always adding them to the MetadataList as MDStrings? Is this a memory savings?
We'd need to hit the context to add the string to a map and unique them there at the same time. I figured that if we load a single function for importing we likely want a tiny portion of the MDString.


https://reviews.llvm.org/D28113