[PATCH] D18259: Use StringRef's in resolved path cache to avoid extra internString lookups

[Pete Cooper via llvm-commits]

pete created this revision.
pete added a reviewer: friss.
pete added a subscriber: llvm-commits.
pete set the repository for this revision to rL LLVM.

Hi Fred

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.

Cheers,
Pete

Repository:
  rL LLVM

http://reviews.llvm.org/D18259

Files:
  tools/dsymutil/DwarfLinker.cpp

Index: tools/dsymutil/DwarfLinker.cpp
===================================================================
--- tools/dsymutil/DwarfLinker.cpp
+++ tools/dsymutil/DwarfLinker.cpp
@@ -315,16 +315,15 @@
   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 @@
   /// @}
 
   /// 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 @@
       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 @@
           // 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 @@
                 &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
           }
         }
       }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D18259.51004.patch
Type: text/x-patch
Size: 3034 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160318/45de829b/attachment.bin>