[PATCH] D43511: [dsymutil] Be smarter in caching calls to realpath

Frederic Riss via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 21 07:25:16 PST 2018


friss added inline comments.


================
Comment at: llvm/tools/dsymutil/DwarfLinker.cpp:125
+private:
+  StringMap<SmallString<256>> ResolvedPaths;
+};
----------------
aprantl wrote:
> aprantl wrote:
> > I don't know how many entries this StringMap is going to have, but this looks expensive in terms of memory usage. Perhaps consider using a StringMap<std::string> instead?
> This is now interning a temporary result that is getting interned again at the call site?
> 
> I think it would make more sense to intern the string that is entered into the StringMap and make it a StringMap<StringRef>. Or both? Fred?
> This is now interning a temporary result that is getting interned again at the call site?
> 
> I think it would make more sense to intern the string that is entered into the StringMap and make it a StringMap<StringRef>. Or both? Fred?

Yes, that's what I had mind, sorry for not having been clear. internString will provide permanent storage. I would never create a std::string, call internString on ResolvedPath and store a StringRef in the StringMap.


https://reviews.llvm.org/D43511





More information about the llvm-commits mailing list