Revise implementation of case-insensitive command names

Store both the as-written and lower-case command names and use
the latter to avoid case-insensitive string comparisons.

With this I obtain 2-6% speed increase (on Windows) for the configure
step with no significant changes in memory usage.  A case-insensitive
comparison is a lot slower than just calling `==` because the operator
will use things like memcmp, so prefer the latter.

The `cmSystemTools::LowerCase` function allocates a new string each time
it is called, so before this change we were allocating in:

* cmMakefile::Configure two times for each function
  (to look for `cmake_minimum_required` and `project`)
* cmMakefile::ExecuteCommand twice by function by calling
  cmState::GetCommand and copying the name

Now we are only allocating once by function instead of four.
This commit is contained in:
Florian Jacomme
2018-05-01 16:17:31 +02:00
committed by Brad King
parent 743f24bac6
commit b1a05d6c76
10 changed files with 64 additions and 41 deletions

View File

@@ -29,10 +29,10 @@ bool cmForEachFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff,
cmMakefile& mf, cmMakefile& mf,
cmExecutionStatus& inStatus) cmExecutionStatus& inStatus)
{ {
if (!cmSystemTools::Strucmp(lff.Name.c_str(), "foreach")) { if (lff.Name.Lower == "foreach") {
// record the number of nested foreach commands // record the number of nested foreach commands
this->Depth++; this->Depth++;
} else if (!cmSystemTools::Strucmp(lff.Name.c_str(), "endforeach")) { } else if (lff.Name.Lower == "endforeach") {
// if this is the endofreach for this statement // if this is the endofreach for this statement
if (!this->Depth) { if (!this->Depth) {
// Remove the function blocker for this scope or bail. // Remove the function blocker for this scope or bail.
@@ -97,7 +97,7 @@ bool cmForEachFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff,
bool cmForEachFunctionBlocker::ShouldRemove(const cmListFileFunction& lff, bool cmForEachFunctionBlocker::ShouldRemove(const cmListFileFunction& lff,
cmMakefile& mf) cmMakefile& mf)
{ {
if (!cmSystemTools::Strucmp(lff.Name.c_str(), "endforeach")) { if (lff.Name.Lower == "endforeach") {
std::vector<std::string> expandedArguments; std::vector<std::string> expandedArguments;
mf.ExpandArguments(lff.Arguments, expandedArguments); mf.ExpandArguments(lff.Arguments, expandedArguments);
// if the endforeach has arguments then make sure // if the endforeach has arguments then make sure

View File

@@ -9,7 +9,6 @@
#include "cmMakefile.h" #include "cmMakefile.h"
#include "cmPolicies.h" #include "cmPolicies.h"
#include "cmState.h" #include "cmState.h"
#include "cmSystemTools.h"
// define the class for function commands // define the class for function commands
class cmFunctionHelperCommand : public cmCommand class cmFunctionHelperCommand : public cmCommand
@@ -128,9 +127,9 @@ bool cmFunctionFunctionBlocker::IsFunctionBlocked(
{ {
// record commands until we hit the ENDFUNCTION // record commands until we hit the ENDFUNCTION
// at the ENDFUNCTION call we shift gears and start looking for invocations // at the ENDFUNCTION call we shift gears and start looking for invocations
if (!cmSystemTools::Strucmp(lff.Name.c_str(), "function")) { if (lff.Name.Lower == "function") {
this->Depth++; this->Depth++;
} else if (!cmSystemTools::Strucmp(lff.Name.c_str(), "endfunction")) { } else if (lff.Name.Lower == "endfunction") {
// if this is the endfunction for this function then execute // if this is the endfunction for this function then execute
if (!this->Depth) { if (!this->Depth) {
// create a new command and add it to cmake // create a new command and add it to cmake
@@ -157,7 +156,7 @@ bool cmFunctionFunctionBlocker::IsFunctionBlocked(
bool cmFunctionFunctionBlocker::ShouldRemove(const cmListFileFunction& lff, bool cmFunctionFunctionBlocker::ShouldRemove(const cmListFileFunction& lff,
cmMakefile& mf) cmMakefile& mf)
{ {
if (!cmSystemTools::Strucmp(lff.Name.c_str(), "endfunction")) { if (lff.Name.Lower == "endfunction") {
std::vector<std::string> expandedArguments; std::vector<std::string> expandedArguments;
mf.ExpandArguments(lff.Arguments, expandedArguments, mf.ExpandArguments(lff.Arguments, expandedArguments,
this->GetStartingContext().FilePath.c_str()); this->GetStartingContext().FilePath.c_str());

View File

@@ -30,9 +30,9 @@ bool cmIfFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff,
cmExecutionStatus& inStatus) cmExecutionStatus& inStatus)
{ {
// we start by recording all the functions // we start by recording all the functions
if (!cmSystemTools::Strucmp(lff.Name.c_str(), "if")) { if (lff.Name.Lower == "if") {
this->ScopeDepth++; this->ScopeDepth++;
} else if (!cmSystemTools::Strucmp(lff.Name.c_str(), "endif")) { } else if (lff.Name.Lower == "endif") {
this->ScopeDepth--; this->ScopeDepth--;
// if this is the endif for this if statement, then start executing // if this is the endif for this if statement, then start executing
if (!this->ScopeDepth) { if (!this->ScopeDepth) {
@@ -48,15 +48,14 @@ bool cmIfFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff,
int scopeDepth = 0; int scopeDepth = 0;
for (cmListFileFunction const& func : this->Functions) { for (cmListFileFunction const& func : this->Functions) {
// keep track of scope depth // keep track of scope depth
if (!cmSystemTools::Strucmp(func.Name.c_str(), "if")) { if (func.Name.Lower == "if") {
scopeDepth++; scopeDepth++;
} }
if (!cmSystemTools::Strucmp(func.Name.c_str(), "endif")) { if (func.Name.Lower == "endif") {
scopeDepth--; scopeDepth--;
} }
// watch for our state change // watch for our state change
if (scopeDepth == 0 && if (scopeDepth == 0 && func.Name.Lower == "else") {
!cmSystemTools::Strucmp(func.Name.c_str(), "else")) {
if (this->ElseSeen) { if (this->ElseSeen) {
cmListFileBacktrace bt = mf.GetBacktrace(func); cmListFileBacktrace bt = mf.GetBacktrace(func);
@@ -76,8 +75,7 @@ bool cmIfFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff,
if (!this->IsBlocking && mf.GetCMakeInstance()->GetTrace()) { if (!this->IsBlocking && mf.GetCMakeInstance()->GetTrace()) {
mf.PrintCommandTrace(func); mf.PrintCommandTrace(func);
} }
} else if (scopeDepth == 0 && } else if (scopeDepth == 0 && func.Name.Lower == "elseif") {
!cmSystemTools::Strucmp(func.Name.c_str(), "elseif")) {
if (this->ElseSeen) { if (this->ElseSeen) {
cmListFileBacktrace bt = mf.GetBacktrace(func); cmListFileBacktrace bt = mf.GetBacktrace(func);
mf.GetCMakeInstance()->IssueMessage( mf.GetCMakeInstance()->IssueMessage(
@@ -163,7 +161,7 @@ bool cmIfFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff,
bool cmIfFunctionBlocker::ShouldRemove(const cmListFileFunction& lff, bool cmIfFunctionBlocker::ShouldRemove(const cmListFileFunction& lff,
cmMakefile&) cmMakefile&)
{ {
if (!cmSystemTools::Strucmp(lff.Name.c_str(), "endif")) { if (lff.Name.Lower == "endif") {
// if the endif has arguments, then make sure // if the endif has arguments, then make sure
// they match the arguments of the matching if // they match the arguments of the matching if
if (lff.Arguments.empty() || lff.Arguments == this->Args) { if (lff.Arguments.empty() || lff.Arguments == this->Args) {

View File

@@ -13,6 +13,14 @@
#include <assert.h> #include <assert.h>
#include <sstream> #include <sstream>
cmCommandContext::cmCommandName& cmCommandContext::cmCommandName::operator=(
std::string const& name)
{
this->Original = name;
this->Lower = cmSystemTools::LowerCase(name);
return *this;
}
struct cmListFileParser struct cmListFileParser
{ {
cmListFileParser(cmListFile* lf, cmListFileBacktrace const& lfbt, cmListFileParser(cmListFile* lf, cmListFileBacktrace const& lfbt,

View File

@@ -23,11 +23,22 @@ class cmMessenger;
struct cmCommandContext struct cmCommandContext
{ {
std::string Name; struct cmCommandName
{
std::string Lower;
std::string Original;
cmCommandName() {}
cmCommandName(std::string const& name) { *this = name; }
cmCommandName& operator=(std::string const& name);
} Name;
long Line; long Line;
cmCommandContext() cmCommandContext()
: Name() : Line(0)
, Line(0) {
}
cmCommandContext(const char* name, int line)
: Name(name)
, Line(line)
{ {
} }
}; };
@@ -81,7 +92,7 @@ public:
cmListFileContext lfc; cmListFileContext lfc;
lfc.FilePath = fileName; lfc.FilePath = fileName;
lfc.Line = lfcc.Line; lfc.Line = lfcc.Line;
lfc.Name = lfcc.Name; lfc.Name = lfcc.Name.Original;
return lfc; return lfc;
} }
}; };

View File

@@ -161,9 +161,9 @@ bool cmMacroFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff,
{ {
// record commands until we hit the ENDMACRO // record commands until we hit the ENDMACRO
// at the ENDMACRO call we shift gears and start looking for invocations // at the ENDMACRO call we shift gears and start looking for invocations
if (!cmSystemTools::Strucmp(lff.Name.c_str(), "macro")) { if (lff.Name.Lower == "macro") {
this->Depth++; this->Depth++;
} else if (!cmSystemTools::Strucmp(lff.Name.c_str(), "endmacro")) { } else if (lff.Name.Lower == "endmacro") {
// if this is the endmacro for this macro then execute // if this is the endmacro for this macro then execute
if (!this->Depth) { if (!this->Depth) {
mf.AppendProperty("MACROS", this->Args[0].c_str()); mf.AppendProperty("MACROS", this->Args[0].c_str());
@@ -191,7 +191,7 @@ bool cmMacroFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff,
bool cmMacroFunctionBlocker::ShouldRemove(const cmListFileFunction& lff, bool cmMacroFunctionBlocker::ShouldRemove(const cmListFileFunction& lff,
cmMakefile& mf) cmMakefile& mf)
{ {
if (!cmSystemTools::Strucmp(lff.Name.c_str(), "endmacro")) { if (lff.Name.Lower == "endmacro") {
std::vector<std::string> expandedArguments; std::vector<std::string> expandedArguments;
mf.ExpandArguments(lff.Arguments, expandedArguments, mf.ExpandArguments(lff.Arguments, expandedArguments,
this->GetStartingContext().FilePath.c_str()); this->GetStartingContext().FilePath.c_str());

View File

@@ -224,7 +224,7 @@ cmListFileBacktrace cmMakefile::GetBacktrace() const
cmListFileBacktrace cmMakefile::GetBacktrace(cmCommandContext const& cc) const cmListFileBacktrace cmMakefile::GetBacktrace(cmCommandContext const& cc) const
{ {
cmListFileContext lfc; cmListFileContext lfc;
lfc.Name = cc.Name; lfc.Name = cc.Name.Original;
lfc.Line = cc.Line; lfc.Line = cc.Line;
lfc.FilePath = this->StateSnapshot.GetExecutionListFile(); lfc.FilePath = this->StateSnapshot.GetExecutionListFile();
return this->Backtrace.Push(lfc); return this->Backtrace.Push(lfc);
@@ -265,7 +265,7 @@ void cmMakefile::PrintCommandTrace(const cmListFileFunction& lff) const
std::ostringstream msg; std::ostringstream msg;
msg << full_path << "(" << lff.Line << "): "; msg << full_path << "(" << lff.Line << "): ";
msg << lff.Name << "("; msg << lff.Name.Original << "(";
bool expand = this->GetCMakeInstance()->GetTraceExpand(); bool expand = this->GetCMakeInstance()->GetTraceExpand();
std::string temp; std::string temp;
for (cmListFileArgument const& arg : lff.Arguments) { for (cmListFileArgument const& arg : lff.Arguments) {
@@ -317,14 +317,13 @@ bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff,
return result; return result;
} }
std::string name = lff.Name;
// Place this call on the call stack. // Place this call on the call stack.
cmMakefileCall stack_manager(this, lff, status); cmMakefileCall stack_manager(this, lff, status);
static_cast<void>(stack_manager); static_cast<void>(stack_manager);
// Lookup the command prototype. // Lookup the command prototype.
if (cmCommand* proto = this->GetState()->GetCommand(name)) { if (cmCommand* proto =
this->GetState()->GetCommandByExactName(lff.Name.Lower)) {
// Clone the prototype. // Clone the prototype.
std::unique_ptr<cmCommand> pcmd(proto->Clone()); std::unique_ptr<cmCommand> pcmd(proto->Clone());
pcmd->SetMakefile(this); pcmd->SetMakefile(this);
@@ -341,7 +340,8 @@ bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff,
if (!invokeSucceeded || hadNestedError) { if (!invokeSucceeded || hadNestedError) {
if (!hadNestedError) { if (!hadNestedError) {
// The command invocation requested that we report an error. // The command invocation requested that we report an error.
std::string const error = name + " " + pcmd->GetError(); std::string const error =
std::string(lff.Name.Original) + " " + pcmd->GetError();
this->IssueMessage(cmake::FATAL_ERROR, error); this->IssueMessage(cmake::FATAL_ERROR, error);
} }
result = false; result = false;
@@ -356,7 +356,7 @@ bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff,
} else { } else {
if (!cmSystemTools::GetFatalErrorOccured()) { if (!cmSystemTools::GetFatalErrorOccured()) {
std::string error = "Unknown CMake command \""; std::string error = "Unknown CMake command \"";
error += lff.Name; error += lff.Name.Original;
error += "\"."; error += "\".";
this->IssueMessage(cmake::FATAL_ERROR, error); this->IssueMessage(cmake::FATAL_ERROR, error);
result = false; result = false;
@@ -1454,7 +1454,7 @@ void cmMakefile::Configure()
bool hasVersion = false; bool hasVersion = false;
// search for the right policy command // search for the right policy command
for (cmListFileFunction const& func : listFile.Functions) { for (cmListFileFunction const& func : listFile.Functions) {
if (cmSystemTools::LowerCase(func.Name) == "cmake_minimum_required") { if (func.Name.Lower == "cmake_minimum_required") {
hasVersion = true; hasVersion = true;
break; break;
} }
@@ -1481,8 +1481,7 @@ void cmMakefile::Configure()
allowedCommands.insert("message"); allowedCommands.insert("message");
isProblem = false; isProblem = false;
for (cmListFileFunction const& func : listFile.Functions) { for (cmListFileFunction const& func : listFile.Functions) {
std::string name = cmSystemTools::LowerCase(func.Name); if (allowedCommands.find(func.Name.Lower) == allowedCommands.end()) {
if (allowedCommands.find(name) == allowedCommands.end()) {
isProblem = true; isProblem = true;
break; break;
} }
@@ -1501,7 +1500,7 @@ void cmMakefile::Configure()
bool hasProject = false; bool hasProject = false;
// search for a project command // search for a project command
for (cmListFileFunction const& func : listFile.Functions) { for (cmListFileFunction const& func : listFile.Functions) {
if (cmSystemTools::LowerCase(func.Name) == "project") { if (func.Name.Lower == "project") {
hasProject = true; hasProject = true;
break; break;
} }
@@ -1509,7 +1508,7 @@ void cmMakefile::Configure()
// if no project command is found, add one // if no project command is found, add one
if (!hasProject) { if (!hasProject) {
cmListFileFunction project; cmListFileFunction project;
project.Name = "PROJECT"; project.Name.Lower = "project";
project.Arguments.emplace_back("Project", cmListFileArgument::Unquoted, project.Arguments.emplace_back("Project", cmListFileArgument::Unquoted,
0); 0);
listFile.Functions.insert(listFile.Functions.begin(), project); listFile.Functions.insert(listFile.Functions.begin(), project);

View File

@@ -462,13 +462,17 @@ void cmState::AddScriptedCommand(std::string const& name, cmCommand* command)
cmCommand* cmState::GetCommand(std::string const& name) const cmCommand* cmState::GetCommand(std::string const& name) const
{ {
std::string sName = cmSystemTools::LowerCase(name); return GetCommandByExactName(cmSystemTools::LowerCase(name));
}
cmCommand* cmState::GetCommandByExactName(std::string const& name) const
{
std::map<std::string, cmCommand*>::const_iterator pos; std::map<std::string, cmCommand*>::const_iterator pos;
pos = this->ScriptedCommands.find(sName); pos = this->ScriptedCommands.find(name);
if (pos != this->ScriptedCommands.end()) { if (pos != this->ScriptedCommands.end()) {
return pos->second; return pos->second;
} }
pos = this->BuiltinCommands.find(sName); pos = this->BuiltinCommands.find(name);
if (pos != this->BuiltinCommands.end()) { if (pos != this->BuiltinCommands.end()) {
return pos->second; return pos->second;
} }

View File

@@ -125,7 +125,11 @@ public:
bool GetIsGeneratorMultiConfig() const; bool GetIsGeneratorMultiConfig() const;
void SetIsGeneratorMultiConfig(bool b); void SetIsGeneratorMultiConfig(bool b);
// Returns a command from its name, case insensitive, or nullptr
cmCommand* GetCommand(std::string const& name) const; cmCommand* GetCommand(std::string const& name) const;
// Returns a command from its name, or nullptr
cmCommand* GetCommandByExactName(std::string const& name) const;
void AddBuiltinCommand(std::string const& name, cmCommand* command); void AddBuiltinCommand(std::string const& name, cmCommand* command);
void AddDisallowedCommand(std::string const& name, cmCommand* command, void AddDisallowedCommand(std::string const& name, cmCommand* command,
cmPolicies::PolicyID policy, const char* message); cmPolicies::PolicyID policy, const char* message);

View File

@@ -28,10 +28,10 @@ bool cmWhileFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff,
cmExecutionStatus& inStatus) cmExecutionStatus& inStatus)
{ {
// at end of for each execute recorded commands // at end of for each execute recorded commands
if (!cmSystemTools::Strucmp(lff.Name.c_str(), "while")) { if (lff.Name.Lower == "while") {
// record the number of while commands past this one // record the number of while commands past this one
this->Depth++; this->Depth++;
} else if (!cmSystemTools::Strucmp(lff.Name.c_str(), "endwhile")) { } else if (lff.Name.Lower == "endwhile") {
// if this is the endwhile for this while loop then execute // if this is the endwhile for this while loop then execute
if (!this->Depth) { if (!this->Depth) {
// Remove the function blocker for this scope or bail. // Remove the function blocker for this scope or bail.
@@ -117,7 +117,7 @@ bool cmWhileFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff,
bool cmWhileFunctionBlocker::ShouldRemove(const cmListFileFunction& lff, bool cmWhileFunctionBlocker::ShouldRemove(const cmListFileFunction& lff,
cmMakefile&) cmMakefile&)
{ {
if (!cmSystemTools::Strucmp(lff.Name.c_str(), "endwhile")) { if (lff.Name.Lower == "endwhile") {
// if the endwhile has arguments, then make sure // if the endwhile has arguments, then make sure
// they match the arguments of the matching while // they match the arguments of the matching while
if (lff.Arguments.empty() || lff.Arguments == this->Args) { if (lff.Arguments.empty() || lff.Arguments == this->Args) {