From ef4e36ad998085770e9230182c3db5240f29dfcf Mon Sep 17 00:00:00 2001
From: Pete Cooper <peter_cooper@apple.com>
Date: Fri, 18 Mar 2016 03:48:09 +0000
Subject: [PATCH] Use StringRef's in resolved path cache to avoid extra
 internString lookups.  NFC.

ResolvedPaths was storing std::string's as a cache. We would then take those strings and look them up in the internString pool to get a unique StringRef for each path.

This patch changes ResolvedPaths to store the StringRef pointing in to the internString pool itself. This way, when getResolvedPath returns a string, we know we have the StringRef we would find in the pool anyway. We can avoid the duplicate memory of the std::string's, and also the time from the lookup.

Unfortunately my profiles show no runtime change here, but it should still save memory allocations which is nice.

Reviewed by Frederic Riss.

Differential Revision: http://reviews.llvm.org/D18259

llvm-svn: 263774
---
 llvm/tools/dsymutil/DwarfLinker.cpp | 27 ++++++++++++++++-----------
 1 file changed, 16 insertions(+), 11 deletions(-)

diff --git a/llvm/tools/dsymutil/DwarfLinker.cpp b/llvm/tools/dsymutil/DwarfLinker.cpp
index 5a7a805..5ac5bd9 100644
--- a/llvm/tools/dsymutil/DwarfLinker.cpp
+++ b/llvm/tools/dsymutil/DwarfLinker.cpp
@@ -315,16 +315,15 @@ public:
   const std::vector<AccelInfo> &getPubtypes() const { return Pubtypes; }
 
   /// Get the full path for file \a FileNum in the line table
-  const char *getResolvedPath(unsigned FileNum) {
+  StringRef getResolvedPath(unsigned FileNum) {
     if (FileNum >= ResolvedPaths.size())
-      return nullptr;
-    return ResolvedPaths[FileNum].size() ? ResolvedPaths[FileNum].c_str()
-                                         : nullptr;
+      return StringRef();
+    return ResolvedPaths[FileNum];
   }
 
   /// Set the fully resolved path for the line-table's file \a FileNum
   /// to \a Path.
-  void setResolvedPath(unsigned FileNum, const std::string &Path) {
+  void setResolvedPath(unsigned FileNum, StringRef Path) {
     if (ResolvedPaths.size() <= FileNum)
       ResolvedPaths.resize(FileNum + 1);
     ResolvedPaths[FileNum] = Path;
@@ -378,7 +377,10 @@ private:
   /// @}
 
   /// Cached resolved paths from the line table.
-  std::vector<std::string> ResolvedPaths;
+  /// Note, the StringRefs here point in to the intern (uniquing) string pool.
+  /// This means that a StringRef returned here doesn't need to then be uniqued
+  /// for the purposes of getting a unique address for each string.
+  std::vector<StringRef> ResolvedPaths;
 
   /// Is this unit subject to the ODR rule?
   bool HasODR;
@@ -1604,7 +1606,6 @@ PointerIntPair<DeclContext *, 1> DeclContextTree::getChildDeclContext(
       Tag != dwarf::DW_TAG_enumeration_type && NameRef.empty())
     return PointerIntPair<DeclContext *, 1>(nullptr);
 
-  std::string File;
   unsigned Line = 0;
   unsigned ByteSize = UINT32_MAX;
 
@@ -1632,6 +1633,7 @@ PointerIntPair<DeclContext *, 1> DeclContextTree::getChildDeclContext(
           // FIXME: Passing U.getOrigUnit().getCompilationDir()
           // instead of "" would allow more uniquing, but for now, do
           // it this way to match dsymutil-classic.
+          std::string File;
           if (LT->getFileNameByIndex(
                   FileNum, "",
                   DILineInfoSpecifier::FileLineInfoKind::AbsoluteFilePath,
@@ -1640,17 +1642,20 @@ PointerIntPair<DeclContext *, 1> DeclContextTree::getChildDeclContext(
                 &U.getOrigUnit(), dwarf::DW_AT_decl_line, 0);
 #ifdef HAVE_REALPATH
             // Cache the resolved paths, because calling realpath is expansive.
-            if (const char *ResolvedPath = U.getResolvedPath(FileNum)) {
-              File = ResolvedPath;
+            StringRef ResolvedPath = U.getResolvedPath(FileNum);
+            if (!ResolvedPath.empty()) {
+              FileRef = ResolvedPath;
             } else {
               char RealPath[PATH_MAX + 1];
               RealPath[PATH_MAX] = 0;
               if (::realpath(File.c_str(), RealPath))
                 File = RealPath;
-              U.setResolvedPath(FileNum, File);
+              FileRef = StringPool.internString(File);
+              U.setResolvedPath(FileNum, FileRef);
             }
-#endif
+#else
             FileRef = StringPool.internString(File);
+#endif
           }
         }
       }
-- 
2.7.4