From 5554910ec2573282a85e61736e96f48e5f550066 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Mon, 10 Feb 2014 01:39:33 -0500 Subject: cmSourceFileLocation: Avoid string allocation in extension checking The substr call was causing excess allocations. Swap the cheaper character check to be before the longer string comparison, now using the prefix checking function. --- Source/cmSourceFileLocation.cxx | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Source/cmSourceFileLocation.cxx b/Source/cmSourceFileLocation.cxx index c050202..9c0b2c5 100644 --- a/Source/cmSourceFileLocation.cxx +++ b/Source/cmSourceFileLocation.cxx @@ -183,8 +183,9 @@ 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; } -- cgit v0.12 From e8e1f3a19fd07ba64381094b98c66f7d07c1746e Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Mon, 3 Feb 2014 16:06:54 -0500 Subject: cmSourceFileLocation: Simplify logic in Matches --- Source/cmSourceFileLocation.cxx | 71 ++++++++++++++++++++--------------------- 1 file changed, 35 insertions(+), 36 deletions(-) diff --git a/Source/cmSourceFileLocation.cxx b/Source/cmSourceFileLocation.cxx index 9c0b2c5..7837738 100644 --- a/Source/cmSourceFileLocation.cxx +++ b/Source/cmSourceFileLocation.cxx @@ -211,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. + // 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 != 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; } @@ -254,28 +251,30 @@ 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. -- cgit v0.12 From b4cb543e0c0648bbd3dbede91f864f89275417c4 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 11 Mar 2014 01:49:11 -0400 Subject: cmSourceFileLocation: Save some string copies --- Source/cmSourceFileLocation.cxx | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Source/cmSourceFileLocation.cxx b/Source/cmSourceFileLocation.cxx index 7837738..1c2454e 100644 --- a/Source/cmSourceFileLocation.cxx +++ b/Source/cmSourceFileLocation.cxx @@ -192,7 +192,7 @@ cmSourceFileLocation // 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& srcExts = mf->GetSourceExtensions(); if(std::find(srcExts.begin(), srcExts.end(), ext) != srcExts.end()) @@ -216,7 +216,7 @@ bool cmSourceFileLocation::Matches(cmSourceFileLocation const& loc) // 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 != loc.Name) + if(this->Name.size() != loc.Name.size() || this->Name != loc.Name) { return false; } @@ -278,10 +278,10 @@ bool cmSourceFileLocation::Matches(cmSourceFileLocation const& loc) 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 && -- cgit v0.12 From 14e7a8ae1c8be09a58156d71b185e2b98d378a07 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 12 Mar 2014 17:04:48 -0400 Subject: cmSourceFileLocation: Return a string reference --- Source/cmSourceFileLocation.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 -- cgit v0.12 From 10baf00f3d14124ee97ca12510595501d66ed70e Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Sat, 8 Feb 2014 21:35:52 -0500 Subject: cmSourceFile: Cache the isUiFile check The filename extension call is expensive, so cache the .ui check. --- Source/cmSourceFile.cxx | 6 +++--- Source/cmSourceFile.h | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/Source/cmSourceFile.cxx b/Source/cmSourceFile.cxx index 0ab30a2..3b130ec 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())); } //---------------------------------------------------------------------------- @@ -297,9 +299,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..42d6f8a 100644 --- a/Source/cmSourceFile.h +++ b/Source/cmSourceFile.h @@ -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); -- cgit v0.12 From 7b8a990424228716cc3161027dcf2c18ef9793b6 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 11 Mar 2014 00:22:02 -0400 Subject: perf: Cache the language property string --- Source/cmSourceFile.cxx | 6 ++++-- Source/cmSourceFile.h | 2 ++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/Source/cmSourceFile.cxx b/Source/cmSourceFile.cxx index 3b130ec..b833d3f 100644 --- a/Source/cmSourceFile.cxx +++ b/Source/cmSourceFile.cxx @@ -40,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) { @@ -56,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; } @@ -93,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; } diff --git a/Source/cmSourceFile.h b/Source/cmSourceFile.h index 42d6f8a..bdcdbfd 100644 --- a/Source/cmSourceFile.h +++ b/Source/cmSourceFile.h @@ -117,6 +117,8 @@ private: void CheckLanguage(std::string const& ext); std::vector Depends; + + static const std::string propLANGUAGE; }; // TODO: Factor out into platform information modules. -- cgit v0.12 From 77b3796581e0b82f0b977418ed52e8c25b96bbd7 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Fri, 21 Mar 2014 22:28:14 -0400 Subject: cmSourceFile: Take a string --- Source/cmSourceFile.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/cmSourceFile.h b/Source/cmSourceFile.h index bdcdbfd..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 &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; } -- cgit v0.12