From f718b30a95e07d72a361d55b7ba495eda5d79680 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 12 Mar 2014 14:23:12 -0400 Subject: [PATCH 1/3] ClearMatches: Only clear matches which were actually set ClearMatches was clearing many variables which were never set in the first place. Instead, store how many matches were made last time and only clear those. It is moved to the cmMakefile class since it is a common utility used by multiple commands. --- Source/cmIfCommand.cxx | 4 ++-- Source/cmMakefile.cxx | 40 ++++++++++++++++++++++++++++++++++ Source/cmMakefile.h | 5 +++++ Source/cmStringCommand.cxx | 44 ++++++-------------------------------- Source/cmStringCommand.h | 2 -- 5 files changed, 53 insertions(+), 42 deletions(-) diff --git a/Source/cmIfCommand.cxx b/Source/cmIfCommand.cxx index 06c4b8993..1141b01b4 100644 --- a/Source/cmIfCommand.cxx +++ b/Source/cmIfCommand.cxx @@ -595,7 +595,7 @@ namespace { def = cmIfCommand::GetVariableOrString(*arg, makefile); const char* rex = (argP2)->c_str(); - cmStringCommand::ClearMatches(makefile); + makefile->ClearMatches(); cmsys::RegularExpression regEntry; if ( !regEntry.compile(rex) ) { @@ -607,7 +607,7 @@ namespace } if (regEntry.find(def)) { - cmStringCommand::StoreMatches(makefile, regEntry); + makefile->StoreMatches(regEntry); *arg = "1"; } else diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 07cfe12e2..262f29d33 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -148,6 +148,8 @@ cmMakefile::cmMakefile(): Internal(new Internals) this->Initialize(); this->PreOrder = false; this->GeneratingBuildSystem = false; + + this->NumLastMatches = 0; } cmMakefile::cmMakefile(const cmMakefile& mf): Internal(new Internals) @@ -196,6 +198,8 @@ cmMakefile::cmMakefile(const cmMakefile& mf): Internal(new Internals) this->CheckSystemVars = mf.CheckSystemVars; this->ListFileStack = mf.ListFileStack; this->OutputToSource = mf.OutputToSource; + + this->NumLastMatches = mf.NumLastMatches; } //---------------------------------------------------------------------------- @@ -4328,6 +4332,42 @@ std::vector cmMakefile::GetQtUiFilesWithOptions() const return this->QtUiFilesWithOptions; } +//---------------------------------------------------------------------------- +void cmMakefile::ClearMatches() +{ + std::stringstream sstr; + for (unsigned int i=0; iNumLastMatches; i++) + { + sstr.str(""); + sstr << "CMAKE_MATCH_" << i; + std::string const& name = sstr.str(); + std::string const& s = this->GetSafeDefinition(name); + if(!s.empty()) + { + this->AddDefinition(name, ""); + this->MarkVariableAsUsed(name); + } + } + this->NumLastMatches = 0; +} + +//---------------------------------------------------------------------------- +void cmMakefile::StoreMatches(cmsys::RegularExpression& re) +{ + for (unsigned int i=0; i<10; i++) + { + std::string m = re.match(i); + if(m.size() > 0) + { + char name[128]; + sprintf(name, "CMAKE_MATCH_%d", i); + this->AddDefinition(name, re.match(i).c_str()); + this->MarkVariableAsUsed(name); + this->NumLastMatches = i + 1; + } + } +} + //---------------------------------------------------------------------------- cmPolicies::PolicyStatus cmMakefile::GetPolicyStatus(cmPolicies::PolicyID id) const diff --git a/Source/cmMakefile.h b/Source/cmMakefile.h index 3bccb63e6..7d1759ea3 100644 --- a/Source/cmMakefile.h +++ b/Source/cmMakefile.h @@ -889,6 +889,9 @@ public: const std::string& feature, std::string *error = 0) const; + void ClearMatches(); + void StoreMatches(cmsys::RegularExpression& re); + protected: // add link libraries and directories to the target void AddGlobalLinkInformation(const std::string& name, cmTarget& target); @@ -1065,6 +1068,8 @@ private: cmSourceFile* source); std::vector QtUiFilesWithOptions; + + unsigned int NumLastMatches; }; //---------------------------------------------------------------------------- diff --git a/Source/cmStringCommand.cxx b/Source/cmStringCommand.cxx index ea762eba7..65912da94 100644 --- a/Source/cmStringCommand.cxx +++ b/Source/cmStringCommand.cxx @@ -305,7 +305,7 @@ bool cmStringCommand::RegexMatch(std::vector const& args) input += args[i]; } - this->ClearMatches(this->Makefile); + this->Makefile->ClearMatches(); // Compile the regular expression. cmsys::RegularExpression re; if(!re.compile(regex.c_str())) @@ -320,7 +320,7 @@ bool cmStringCommand::RegexMatch(std::vector const& args) std::string output; if(re.find(input.c_str())) { - this->StoreMatches(this->Makefile, re); + this->Makefile->StoreMatches(re); std::string::size_type l = re.start(); std::string::size_type r = re.end(); if(r-l == 0) @@ -354,7 +354,7 @@ bool cmStringCommand::RegexMatchAll(std::vector const& args) input += args[i]; } - this->ClearMatches(this->Makefile); + this->Makefile->ClearMatches(); // Compile the regular expression. cmsys::RegularExpression re; if(!re.compile(regex.c_str())) @@ -371,7 +371,7 @@ bool cmStringCommand::RegexMatchAll(std::vector const& args) const char* p = input.c_str(); while(re.find(p)) { - this->StoreMatches(this->Makefile, re); + this->Makefile->StoreMatches(re); std::string::size_type l = re.start(); std::string::size_type r = re.end(); if(r-l == 0) @@ -458,7 +458,7 @@ bool cmStringCommand::RegexReplace(std::vector const& args) input += args[i]; } - this->ClearMatches(this->Makefile); + this->Makefile->ClearMatches(); // Compile the regular expression. cmsys::RegularExpression re; if(!re.compile(regex.c_str())) @@ -475,7 +475,7 @@ bool cmStringCommand::RegexReplace(std::vector const& args) std::string::size_type base = 0; while(re.find(input.c_str()+base)) { - this->StoreMatches(this->Makefile, re); + this->Makefile->StoreMatches(re); std::string::size_type l2 = re.start(); std::string::size_type r = re.end(); @@ -535,38 +535,6 @@ bool cmStringCommand::RegexReplace(std::vector const& args) return true; } -//---------------------------------------------------------------------------- -void cmStringCommand::ClearMatches(cmMakefile* mf) -{ - for (unsigned int i=0; i<10; i++) - { - char name[128]; - sprintf(name, "CMAKE_MATCH_%d", i); - const char* s = mf->GetDefinition(name); - if(s && *s != 0) - { - mf->AddDefinition(name, ""); - mf->MarkVariableAsUsed(name); - } - } -} - -//---------------------------------------------------------------------------- -void cmStringCommand::StoreMatches(cmMakefile* mf,cmsys::RegularExpression& re) -{ - for (unsigned int i=0; i<10; i++) - { - std::string m = re.match(i); - if(m.size() > 0) - { - char name[128]; - sprintf(name, "CMAKE_MATCH_%d", i); - mf->AddDefinition(name, re.match(i).c_str()); - mf->MarkVariableAsUsed(name); - } - } -} - //---------------------------------------------------------------------------- bool cmStringCommand::HandleFindCommand(std::vector const& args) diff --git a/Source/cmStringCommand.h b/Source/cmStringCommand.h index 51069e709..8292e643d 100644 --- a/Source/cmStringCommand.h +++ b/Source/cmStringCommand.h @@ -53,8 +53,6 @@ public: virtual std::string GetName() const { return "string";} cmTypeMacro(cmStringCommand, cmCommand); - static void ClearMatches(cmMakefile* mf); - static void StoreMatches(cmMakefile* mf, cmsys::RegularExpression& re); protected: bool HandleConfigureCommand(std::vector const& args); bool HandleAsciiCommand(std::vector const& args); From ef62fbad55deedd4b985f0900f1ee983eaa0072d Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 12 Mar 2014 14:25:59 -0400 Subject: [PATCH 2/3] ClearMatches: Store match variable names statically Constructing the names and then turning them into a std::string is non-negligible in performance testing. --- Source/cmMakefile.cxx | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 262f29d33..b71e11339 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -4332,20 +4332,30 @@ std::vector cmMakefile::GetQtUiFilesWithOptions() const return this->QtUiFilesWithOptions; } +static std::string matchVariables[] = { + "CMAKE_MATCH_0", + "CMAKE_MATCH_1", + "CMAKE_MATCH_2", + "CMAKE_MATCH_3", + "CMAKE_MATCH_4", + "CMAKE_MATCH_5", + "CMAKE_MATCH_6", + "CMAKE_MATCH_7", + "CMAKE_MATCH_8", + "CMAKE_MATCH_9" +}; + //---------------------------------------------------------------------------- void cmMakefile::ClearMatches() { - std::stringstream sstr; for (unsigned int i=0; iNumLastMatches; i++) { - sstr.str(""); - sstr << "CMAKE_MATCH_" << i; - std::string const& name = sstr.str(); - std::string const& s = this->GetSafeDefinition(name); + std::string const& var = matchVariables[i]; + std::string const& s = this->GetSafeDefinition(var); if(!s.empty()) { - this->AddDefinition(name, ""); - this->MarkVariableAsUsed(name); + this->AddDefinition(var, ""); + this->MarkVariableAsUsed(var); } } this->NumLastMatches = 0; @@ -4359,10 +4369,9 @@ void cmMakefile::StoreMatches(cmsys::RegularExpression& re) std::string m = re.match(i); if(m.size() > 0) { - char name[128]; - sprintf(name, "CMAKE_MATCH_%d", i); - this->AddDefinition(name, re.match(i).c_str()); - this->MarkVariableAsUsed(name); + std::string const& var = matchVariables[i]; + this->AddDefinition(var, re.match(i).c_str()); + this->MarkVariableAsUsed(var); this->NumLastMatches = i + 1; } } From 3f51752264bc1243fa2e56da41131ac363d3bd85 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 12 Mar 2014 14:26:45 -0400 Subject: [PATCH 3/3] StoreMatches: Minor cleanups --- Source/cmMakefile.cxx | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index b71e11339..de329f0c6 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -4366,11 +4366,11 @@ void cmMakefile::StoreMatches(cmsys::RegularExpression& re) { for (unsigned int i=0; i<10; i++) { - std::string m = re.match(i); - if(m.size() > 0) + std::string const& m = re.match(i); + if(!m.empty()) { std::string const& var = matchVariables[i]; - this->AddDefinition(var, re.match(i).c_str()); + this->AddDefinition(var, m.c_str()); this->MarkVariableAsUsed(var); this->NumLastMatches = i + 1; }