[cfe-commits] r173542 - Since we're stuck with realpath for the header <-> module mapping,

Chad Rosier mcrosier at apple.com
Mon Mar 4 12:15:48 PST 2013


On Jan 25, 2013, at 4:55 PM, Douglas Gregor <dgregor at apple.com> wrote:

> Author: dgregor
> Date: Fri Jan 25 18:55:12 2013
> New Revision: 173542
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=173542&view=rev
> Log:
> Since we're stuck with realpath for the header <-> module mapping,
> factor the realpath calls into FileManager::getCanonicalName() so we
> can cache the results of this epically slow operation. 5% speedup on
> my modules test, and realpath drops out of the profile.
> 
> Modified:
>    cfe/trunk/include/clang/Basic/FileManager.h
>    cfe/trunk/lib/Basic/FileManager.cpp
>    cfe/trunk/lib/Lex/HeaderSearch.cpp
>    cfe/trunk/lib/Lex/ModuleMap.cpp
> 
> Modified: cfe/trunk/include/clang/Basic/FileManager.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=173542&r1=173541&r2=173542&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/FileManager.h (original)
> +++ cfe/trunk/include/clang/Basic/FileManager.h Fri Jan 25 18:55:12 2013
> @@ -17,6 +17,7 @@
> 
> #include "clang/Basic/FileSystemOptions.h"
> #include "clang/Basic/LLVM.h"
> +#include "llvm/ADT/DenseMap.h"
> #include "llvm/ADT/IntrusiveRefCntPtr.h"
> #include "llvm/ADT/OwningPtr.h"
> #include "llvm/ADT/SmallVector.h"
> @@ -152,6 +153,12 @@ class FileManager : public RefCountedBas
>   /// \see SeenDirEntries
>   llvm::StringMap<FileEntry*, llvm::BumpPtrAllocator> SeenFileEntries;
> 
> +  /// \brief The canonical names of directories.
> +  llvm::DenseMap<const DirectoryEntry *, llvm::StringRef> CanonicalDirNames;
> +
> +  /// \brief Storage for canonical names that we have computed.
> +  llvm::BumpPtrAllocator CanonicalNameStorage;
> +
>   /// \brief Each FileEntry we create is assigned a unique ID #.
>   ///
>   unsigned NextFileUID;
> @@ -257,6 +264,13 @@ public:
>   static void modifyFileEntry(FileEntry *File, off_t Size,
>                               time_t ModificationTime);
> 
> +  /// \brief Retrieve the canonical name for a given directory.
> +  ///
> +  /// This is a very expensive operation, despite its results being cached,
> +  /// and should only be used when the physical layout of the file system is
> +  /// required, which is (almost) never.
> +  StringRef getCanonicalName(const DirectoryEntry *Dir);
> +
>   void PrintStats() const;
> };
> 
> 
> Modified: cfe/trunk/lib/Basic/FileManager.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=173542&r1=173541&r2=173542&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Basic/FileManager.cpp (original)
> +++ cfe/trunk/lib/Basic/FileManager.cpp Fri Jan 25 18:55:12 2013
> @@ -40,6 +40,11 @@
> #define S_ISFIFO(x) (0)
> #endif
> #endif
> +#if defined(LLVM_ON_UNIX)
> +#if defined(__linux__)
> +#include <linux/limits.h>
> +#endif
> +#endif
> using namespace clang;
> 
> // FIXME: Enhance libsystem to support inode and other fields.
> @@ -620,6 +625,29 @@ void FileManager::modifyFileEntry(FileEn
>   File->ModTime = ModificationTime;
> }
> 
> +StringRef FileManager::getCanonicalName(const DirectoryEntry *Dir) {
> +  // FIXME: use llvm::sys::fs::canonical() when it gets implemented
> +#ifdef LLVM_ON_UNIX
> +  llvm::DenseMap<const DirectoryEntry *, llvm::StringRef>::iterator Known
> +    = CanonicalDirNames.find(Dir);
> +  if (Known != CanonicalDirNames.end())
> +    return Known->second;
> +
> +  StringRef CanonicalName(Dir->getName());
> +  char CanonicalNameBuf[PATH_MAX];
> +  if (realpath(Dir->getName(), CanonicalNameBuf)) {
> +    unsigned Len = strlen(CanonicalNameBuf);
> +    char *Mem = static_cast<char *>(CanonicalNameStorage.Allocate(Len, 1));
> +    memcpy(Mem, CanonicalNameBuf, Len);
> +    CanonicalName = StringRef(Mem, Len);
> +  }
> +
> +  CanonicalDirNames.insert(std::make_pair(Dir, CanonicalName));
> +  return CanonicalName;
> +#else
> +  return StringRef(Dir->getName());
> +#endif
> +}
> 
> void FileManager::PrintStats() const {
>   llvm::errs() << "\n*** File Manager Stats:\n";
> 
> Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=173542&r1=173541&r2=173542&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
> +++ cfe/trunk/lib/Lex/HeaderSearch.cpp Fri Jan 25 18:55:12 2013
> @@ -268,6 +268,10 @@ const FileEntry *DirectoryLookup::Lookup
>   return Result;
> }
> 
> +/// FIXME: HACK HACK HACK!
> +static llvm::DenseMap<const DirectoryEntry *, const DirectoryEntry *>
> +  TopFrameworkDirs;
> +

Hi Doug,
Is the TopFrameworkDirs map actually used?

 Chad


> /// \brief Given a framework directory, find the top-most framework directory.
> ///
> /// \param FileMgr The file manager to use for directory lookups.
> @@ -280,7 +284,6 @@ getTopFrameworkDir(FileManager &FileMgr,
>   assert(llvm::sys::path::extension(DirName) == ".framework" &&
>          "Not a framework directory");
> 
> -#ifdef LLVM_ON_UNIX
>   // Note: as an egregious but useful hack we use the real path here, because
>   // frameworks moving between top-level frameworks to embedded frameworks tend
>   // to be symlinked, and we base the logical structure of modules on the
> @@ -295,12 +298,8 @@ getTopFrameworkDir(FileManager &FileMgr,
>   //
>   // Similar issues occur when a top-level framework has moved into an
>   // embedded framework.
> -  char RealDirName[PATH_MAX];
> -  if (realpath(DirName.str().c_str(), RealDirName))
> -    DirName = RealDirName;
> -#endif
> -
>   const DirectoryEntry *TopFrameworkDir = FileMgr.getDirectory(DirName);
> +  DirName = FileMgr.getCanonicalName(TopFrameworkDir);
>   do {
>     // Get the parent directory name.
>     DirName = llvm::sys::path::parent_path(DirName);
> 
> Modified: cfe/trunk/lib/Lex/ModuleMap.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=173542&r1=173541&r2=173542&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Lex/ModuleMap.cpp (original)
> +++ cfe/trunk/lib/Lex/ModuleMap.cpp Fri Jan 25 18:55:12 2013
> @@ -163,20 +163,12 @@ Module *ModuleMap::findModuleForHeader(c
> 
>   const DirectoryEntry *Dir = File->getDir();
>   SmallVector<const DirectoryEntry *, 2> SkippedDirs;
> -#ifdef LLVM_ON_UNIX
> +
>   // Note: as an egregious but useful hack we use the real path here, because
>   // frameworks moving from top-level frameworks to embedded frameworks tend
>   // to be symlinked from the top-level location to the embedded location,
>   // and we need to resolve lookups as if we had found the embedded location.
> -  char RealDirName[PATH_MAX];
> -  StringRef DirName;
> -  if (realpath(Dir->getName(), RealDirName))
> -    DirName = RealDirName;
> -  else
> -    DirName = Dir->getName();
> -#else
> -  StringRef DirName = Dir->getName();
> -#endif
> +  StringRef DirName = SourceMgr->getFileManager().getCanonicalName(Dir);
> 
>   // Keep walking up the directory hierarchy, looking for a directory with
>   // an umbrella header.
> @@ -420,16 +412,13 @@ ModuleMap::inferFrameworkModule(StringRe
>   // a framework module, do so.
>   if (!Parent) {
>     // Determine whether we're allowed to infer a module map.
> -    StringRef FrameworkDirName = FrameworkDir->getName();
> -#ifdef LLVM_ON_UNIX
> +
>     // Note: as an egregious but useful hack we use the real path here, because
>     // we might be looking at an embedded framework that symlinks out to a
>     // top-level framework, and we need to infer as if we were naming the
>     // top-level framework.
> -    char RealFrameworkDirName[PATH_MAX];
> -    if (realpath(FrameworkDir->getName(), RealFrameworkDirName))
> -      FrameworkDirName = RealFrameworkDirName;
> -#endif
> +    StringRef FrameworkDirName
> +      = SourceMgr->getFileManager().getCanonicalName(FrameworkDir);
> 
>     bool canInfer = false;
>     if (llvm::sys::path::has_parent_path(FrameworkDirName)) {
> @@ -527,29 +516,23 @@ ModuleMap::inferFrameworkModule(StringRe
>       // check whether it is actually a subdirectory of the parent directory.
>       // This will not be the case if the 'subframework' is actually a symlink
>       // out to a top-level framework.
> -#ifdef LLVM_ON_UNIX
> -      char RealSubframeworkDirName[PATH_MAX];
> -      if (realpath(Dir->path().c_str(), RealSubframeworkDirName)) {
> -        StringRef SubframeworkDirName = RealSubframeworkDirName;
> -
> -        bool FoundParent = false;
> -        do {
> -          // Get the parent directory name.
> -          SubframeworkDirName
> -            = llvm::sys::path::parent_path(SubframeworkDirName);
> -          if (SubframeworkDirName.empty())
> -            break;
> -
> -          if (FileMgr.getDirectory(SubframeworkDirName) == FrameworkDir) {
> -            FoundParent = true;
> -            break;
> -          }
> -        } while (true);
> +      StringRef SubframeworkDirName = FileMgr.getCanonicalName(SubframeworkDir);
> +      bool FoundParent = false;
> +      do {
> +        // Get the parent directory name.
> +        SubframeworkDirName
> +          = llvm::sys::path::parent_path(SubframeworkDirName);
> +        if (SubframeworkDirName.empty())
> +          break;
> +
> +        if (FileMgr.getDirectory(SubframeworkDirName) == FrameworkDir) {
> +          FoundParent = true;
> +          break;
> +        }
> +      } while (true);
> 
> -        if (!FoundParent)
> -          continue;
> -      }
> -#endif
> +      if (!FoundParent)
> +        continue;
> 
>       // FIXME: Do we want to warn about subframeworks without umbrella headers?
>       SmallString<32> NameBuf;
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130304/ae2a9f76/attachment.html>


More information about the cfe-commits mailing list