Use StringRef's in resolved path cache to avoid extra internString lookups

[pete]

Summary

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

[friss]

LGTM, thanks Pete!

[pete]

Thanks for the quick review! r263774

Pete

llvm/trunk/tools/dsymutil/DwarfLinker.cpp

AccelInfo(StringRef Name, const DIE *Die, uint32_t NameOffset,
         : Name(Name), Die(Die), NameOffset(NameOffset),
           SkipPubSection(SkipPubSection) {}
   };
 
   const std::vector<AccelInfo> &getPubnames() const { return Pubnames; }
   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;
   }
 
 private:
   DWARFUnit &OrigUnit;
   unsigned ID;
private:
   /// \brief Accelerator entries for the unit, both for the pub*
   /// sections and the apple* ones.
   /// @{
   std::vector<AccelInfo> Pubnames;
   std::vector<AccelInfo> Pubtypes;
   /// @}
 
   /// 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;
   /// Did a DIE actually contain a valid reloc?
   bool HasInterestingContent;
   /// If this is a Clang module, this holds the module's name.
   std::string ClangModuleName;
 };
PointerIntPair<DeclContext *, 1> DeclContextTree::getChildDeclContext(
   else
     ShortNameRef = NameRef;
 
   if (Tag != dwarf::DW_TAG_class_type && Tag != dwarf::DW_TAG_structure_type &&
       Tag != dwarf::DW_TAG_union_type &&
       Tag != dwarf::DW_TAG_enumeration_type && NameRef.empty())
     return PointerIntPair<DeclContext *, 1>(nullptr);
 
-  std::string File;
   unsigned Line = 0;
   unsigned ByteSize = UINT32_MAX;
 
   if (!InClangModule) {
     // Gather some discriminating data about the DeclContext we will be
     // creating: File, line number and byte size. This shouldn't be
     // necessary, because the ODR is just about names, but given that we
     // do some approximations with overloaded functions and anonymous
if (Tag != dwarf::DW_TAG_namespace || !Name) {
           // unique anything in anonymous namespaces, but if we do, then
           // verify that the file and line correspond.
           if (!Name && Tag == dwarf::DW_TAG_namespace)
             FileNum = 1;
 
           // 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,
                   File)) {
             Line = DIE->getAttributeValueAsUnsignedConstant(
                 &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
           }
         }
       }
     }
   }
 
   if (!Line && NameRef.empty())
     return PointerIntPair<DeclContext *, 1>(nullptr);