[llvm] r263774 - Use StringRef's in resolved path cache to avoid extra internString lookups. NFC.

[Pete Cooper via llvm-commits]

Author: pete
Date: Thu Mar 17 22:48:09 2016
New Revision: 263774

URL: http://llvm.org/viewvc/llvm-project?rev=263774&view=rev
Log:
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

Modified:
    llvm/trunk/tools/dsymutil/DwarfLinker.cpp

Modified: llvm/trunk/tools/dsymutil/DwarfLinker.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/dsymutil/DwarfLinker.cpp?rev=263774&r1=263773&r2=263774&view=diff
==============================================================================
--- llvm/trunk/tools/dsymutil/DwarfLinker.cpp (original)
+++ llvm/trunk/tools/dsymutil/DwarfLinker.cpp Thu Mar 17 22:48:09 2016
@@ -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> DeclCon
       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> DeclCon
           // 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> DeclCon
                 &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
           }
         }
       }