echo/CMake
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Revise implementation of case-insensitive command names

Browse Source 
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
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
Source/cmForEachCommand.cxx
View File

@ -29,10 +29,10 @@ bool cmForEachFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff,
cmMakefile& mf,
cmExecutionStatus& inStatus)
{
if (!cmSystemTools::Strucmp(lff.Name.c_str(), "foreach")) {
if (lff.Name.Lower == "foreach") {
// record the number of nested foreach commands
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->Depth) {
// 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,
cmMakefile& mf)
{
if (!cmSystemTools::Strucmp(lff.Name.c_str(), "endforeach")) {
if (lff.Name.Lower == "endforeach") {
std::vector<std::string> expandedArguments;
mf.ExpandArguments(lff.Arguments, expandedArguments);
// if the endforeach has arguments then make sure

7
Source/cmFunctionCommand.cxx
View File

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

16
Source/cmIfCommand.cxx
View File

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

8
Source/cmListFileCache.cxx
View File

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

19
Source/cmListFileCache.h
View File

@ -23,11 +23,22 @@ class cmMessenger;
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;
cmCommandContext()
: Name()
, Line(0)
: Line(0)
{
}
cmCommandContext(const char* name, int line)
: Name(name)
, Line(line)
{
}
};
@ -81,7 +92,7 @@ public:
cmListFileContext lfc;
lfc.FilePath = fileName;
lfc.Line = lfcc.Line;
lfc.Name = lfcc.Name;
lfc.Name = lfcc.Name.Original;
return lfc;
}
};

6
Source/cmMacroCommand.cxx
View File

@ -161,9 +161,9 @@ bool cmMacroFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff,
{
// record commands until we hit the ENDMACRO
// 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++;
} 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->Depth) {
mf.AppendProperty("MACROS", this->Args[0].c_str());
@ -191,7 +191,7 @@ bool cmMacroFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff,
bool cmMacroFunctionBlocker::ShouldRemove(const cmListFileFunction& lff,
cmMakefile& mf)
{
if (!cmSystemTools::Strucmp(lff.Name.c_str(), "endmacro")) {
if (lff.Name.Lower == "endmacro") {
std::vector<std::string> expandedArguments;
mf.ExpandArguments(lff.Arguments, expandedArguments,
this->GetStartingContext().FilePath.c_str());

23
Source/cmMakefile.cxx
View File

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

10
Source/cmState.cxx
View File

@ -462,13 +462,17 @@ void cmState::AddScriptedCommand(std::string const& name, cmCommand* command)
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;
pos = this->ScriptedCommands.find(sName);
pos = this->ScriptedCommands.find(name);
if (pos != this->ScriptedCommands.end()) {
return pos->second;
}
pos = this->BuiltinCommands.find(sName);
pos = this->BuiltinCommands.find(name);
if (pos != this->BuiltinCommands.end()) {
return pos->second;
}

4
Source/cmState.h
View File

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

6
Source/cmWhileCommand.cxx
View File

@ -28,10 +28,10 @@ bool cmWhileFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff,
cmExecutionStatus& inStatus)
{
// 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
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->Depth) {
// 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,
cmMakefile&)
{
if (!cmSystemTools::Strucmp(lff.Name.c_str(), "endwhile")) {
if (lff.Name.Lower == "endwhile") {
// if the endwhile has arguments, then make sure
// they match the arguments of the matching while
if (lff.Arguments.empty() || lff.Arguments == this->Args) {