Merge topic 'dev/source-file-performance'

77b37965 cmSourceFile: Take a string
7b8a9904 perf: Cache the language property string
10baf00f cmSourceFile: Cache the isUiFile check
14e7a8ae cmSourceFileLocation: Return a string reference
b4cb543e cmSourceFileLocation: Save some string copies
e8e1f3a1 cmSourceFileLocation: Simplify logic in Matches
5554910e cmSourceFileLocation: Avoid string allocation in extension checking

4 files changed, 56 insertions, 51 deletions

diff --git a/Source/cmSourceFile.cxx b/Source/cmSourceFile.cxx
index 0ab30a2..b833d3f 100644
--- a/Source/cmSourceFile.cxx
+++ b/Source/cmSourceFile.cxx
@@ -24,6 +24,8 @@ cmSourceFile::cmSourceFile(cmMakefile* mf, const std::string& name):
   this->CustomCommand = 0;
   this->Properties.SetCMakeInstance(mf->GetCMakeInstance());
   this->FindFullPathFailed = false;
+  this->IsUiFile = ("ui" ==
+          cmSystemTools::GetFilenameLastExtension(this->Location.GetName()));
 }
 
 //----------------------------------------------------------------------------
@@ -38,6 +40,8 @@ std::string const& cmSourceFile::GetExtension() const
   return this->Extension;
 }
 
+const std::string cmSourceFile::propLANGUAGE = "LANGUAGE";
+
 //----------------------------------------------------------------------------
 void cmSourceFile::SetObjectLibrary(std::string const& objlib)
 {
@@ -54,7 +58,7 @@ std::string cmSourceFile::GetObjectLibrary() const
 std::string cmSourceFile::GetLanguage()
 {
   // If the language was set explicitly by the user then use it.
-  if(const char* lang = this->GetProperty("LANGUAGE"))
+  if(const char* lang = this->GetProperty(propLANGUAGE))
     {
     return lang;
     }
@@ -91,7 +95,7 @@ std::string cmSourceFile::GetLanguage()
 std::string cmSourceFile::GetLanguage() const
 {
   // If the language was set explicitly by the user then use it.
-  if(const char* lang = this->GetProperty("LANGUAGE"))
+  if(const char* lang = this->GetProperty(propLANGUAGE))
     {
     return lang;
     }
@@ -297,9 +301,7 @@ void cmSourceFile::SetProperty(const std::string& prop, const char* value)
 {
   this->Properties.SetProperty(prop, value, cmProperty::SOURCE_FILE);
 
-  std::string ext =
-          cmSystemTools::GetFilenameLastExtension(this->Location.GetName());
-  if (ext == ".ui")
+  if (this->IsUiFile)
     {
     cmMakefile const* mf = this->Location.GetMakefile();
     if (prop == "AUTOUIC_OPTIONS")
diff --git a/Source/cmSourceFile.h b/Source/cmSourceFile.h
index a1d9de1..f898260 100644
--- a/Source/cmSourceFile.h
+++ b/Source/cmSourceFile.h
@@ -86,7 +86,7 @@ public:
    * Return the vector that holds the list of dependencies
    */
   const std::vector<std::string> &GetDepends() const {return this->Depends;}
-  void AddDepend(const char* d) { this->Depends.push_back(d); }
+  void AddDepend(const std::string& d) { this->Depends.push_back(d); }
 
   // Get the properties
   cmPropertyMap &GetProperties() { return this->Properties; }
@@ -109,6 +109,7 @@ private:
   std::string FullPath;
   bool FindFullPathFailed;
   std::string ObjectLibrary;
+  bool IsUiFile;
 
   bool FindFullPath(std::string* error);
   bool TryFullPath(const std::string& path, const std::string& ext);
@@ -116,6 +117,8 @@ private:
   void CheckLanguage(std::string const& ext);
 
   std::vector<std::string> Depends;
+
+  static const std::string propLANGUAGE;
 };
 
 // TODO: Factor out into platform information modules.
diff --git a/Source/cmSourceFileLocation.cxx b/Source/cmSourceFileLocation.cxx
index c050202..1c2454e 100644
--- a/Source/cmSourceFileLocation.cxx
+++ b/Source/cmSourceFileLocation.cxx
@@ -183,15 +183,16 @@ cmSourceFileLocation
   // Check if loc's name could possibly be extended to our name by
   // adding an extension.
   if(!(this->Name.size() > loc.Name.size() &&
-       this->Name.substr(0, loc.Name.size()) == loc.Name &&
-       this->Name[loc.Name.size()] == '.'))
+       this->Name[loc.Name.size()] == '.' &&
+       cmHasLiteralPrefixImpl(this->Name.c_str(),
+                              loc.Name.c_str(), loc.Name.size())))
     {
     return false;
     }
 
   // Only a fixed set of extensions will be tried to match a file on
   // disk.  One of these must match if loc refers to this source file.
-  std::string ext = this->Name.substr(loc.Name.size()+1);
+  std::string const& ext = this->Name.substr(loc.Name.size()+1);
   cmMakefile const* mf = this->Makefile;
   const std::vector<std::string>& srcExts = mf->GetSourceExtensions();
   if(std::find(srcExts.begin(), srcExts.end(), ext) != srcExts.end())
@@ -210,36 +211,33 @@ cmSourceFileLocation
 bool cmSourceFileLocation::Matches(cmSourceFileLocation const& loc)
 {
   assert(this->Makefile);
-  if(this->AmbiguousExtension && loc.AmbiguousExtension)
+  if(this->AmbiguousExtension == loc.AmbiguousExtension)
     {
-    // Both extensions are ambiguous.  Since only the old fixed set of
-    // extensions will be tried, the names must match at this point to
-    // be the same file.
-    if(this->Name != loc.Name)
+    // Both extensions are similarly ambiguous.  Since only the old fixed set
+    // of extensions will be tried, the names must match at this point to be
+    // the same file.
+    if(this->Name.size() != loc.Name.size() || this->Name != loc.Name)
       {
       return false;
       }
     }
-  else if(this->AmbiguousExtension)
+  else
     {
-    // Only "this" extension is ambiguous.
-    if(!loc.MatchesAmbiguousExtension(*this))
+    const cmSourceFileLocation* loc1;
+    const cmSourceFileLocation* loc2;
+    if(this->AmbiguousExtension)
       {
-      return false;
+      // Only "this" extension is ambiguous.
+      loc1 = &loc;
+      loc2 = this;
       }
-    }
-  else if(loc.AmbiguousExtension)
-    {
-    // Only "loc" extension is ambiguous.
-    if(!this->MatchesAmbiguousExtension(loc))
+    else
       {
-      return false;
+      // Only "loc" extension is ambiguous.
+      loc1 = this;
+      loc2 = &loc;
       }
-    }
-  else
-    {
-    // Neither extension is ambiguous.
-    if(this->Name != loc.Name)
+    if(!loc1->MatchesAmbiguousExtension(*loc2))
       {
       return false;
       }
@@ -253,35 +251,37 @@ bool cmSourceFileLocation::Matches(cmSourceFileLocation const& loc)
       return false;
       }
     }
-  else if(this->AmbiguousDirectory && loc.AmbiguousDirectory &&
-          this->Makefile == loc.Makefile)
+  else if(this->AmbiguousDirectory && loc.AmbiguousDirectory)
     {
-    // Both sides have directories relative to the same location.
-    if(this->Directory != loc.Directory)
+    if (this->Makefile == loc.Makefile)
+      {
+      // Both sides have directories relative to the same location.
+      if(this->Directory != loc.Directory)
+        {
+        return false;
+        }
+      }
+    else
       {
+      // Each side has a directory relative to a different location.
+      // This can occur when referencing a source file from a different
+      // directory.  This is not yet allowed.
+      this->Makefile->IssueMessage(
+        cmake::INTERNAL_ERROR,
+        "Matches error: Each side has a directory relative to a different "
+        "location. This can occur when referencing a source file from a "
+        "different directory.  This is not yet allowed."
+        );
       return false;
       }
     }
-  else if(this->AmbiguousDirectory && loc.AmbiguousDirectory)
-    {
-    // Each side has a directory relative to a different location.
-    // This can occur when referencing a source file from a different
-    // directory.  This is not yet allowed.
-    this->Makefile->IssueMessage(
-      cmake::INTERNAL_ERROR,
-      "Matches error: Each side has a directory relative to a different "
-      "location. This can occur when referencing a source file from a "
-      "different directory.  This is not yet allowed."
-      );
-    return false;
-    }
   else if(this->AmbiguousDirectory)
     {
     // Compare possible directory combinations.
-    std::string srcDir =
+    std::string const& srcDir =
       cmSystemTools::CollapseFullPath(
         this->Directory.c_str(), this->Makefile->GetCurrentDirectory());
-    std::string binDir =
+    std::string const& binDir =
       cmSystemTools::CollapseFullPath(
         this->Directory.c_str(), this->Makefile->GetCurrentOutputDirectory());
     if(srcDir != loc.Directory &&
@@ -293,10 +293,10 @@ bool cmSourceFileLocation::Matches(cmSourceFileLocation const& loc)
   else if(loc.AmbiguousDirectory)
     {
     // Compare possible directory combinations.
-    std::string srcDir =
+    std::string const& srcDir =
       cmSystemTools::CollapseFullPath(
         loc.Directory.c_str(), loc.Makefile->GetCurrentDirectory());
-    std::string binDir =
+    std::string const& binDir =
       cmSystemTools::CollapseFullPath(
         loc.Directory.c_str(), loc.Makefile->GetCurrentOutputDirectory());
     if(srcDir != this->Directory &&
diff --git a/Source/cmSourceFileLocation.h b/Source/cmSourceFileLocation.h
index c37fb1d..af3651a 100644
--- a/Source/cmSourceFileLocation.h
+++ b/Source/cmSourceFileLocation.h
@@ -71,7 +71,7 @@ public:
    * Otherwise it will be a relative path (possibly empty) that is
    * either with respect to the source or build tree.
    */
-  const char* GetDirectory() const { return this->Directory.c_str(); }
+  const std::string& GetDirectory() const { return this->Directory; }
 
   /**
    * Get the file name as best is currently known.  If