From c0e7a137025789c41349ae0935609e7bd083587a Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 23 Jan 2018 18:29:13 -0500 Subject: cmAddCustomCommandCommand: store keywords in strings Callgrind indicated that `strlen` was being called a lot of times here due to the string comparisons. Since keywords are "sparse" in `add_custom_command`, use a hash comparison to handle keywords and then use strings for comparison since they have a built-in length parameter. --- Source/cmAddCustomCommandCommand.cxx | 148 +++++++++++++++++++++++------------ 1 file changed, 99 insertions(+), 49 deletions(-) diff --git a/Source/cmAddCustomCommandCommand.cxx b/Source/cmAddCustomCommandCommand.cxx index 14dfdae..3186583 100644 --- a/Source/cmAddCustomCommandCommand.cxx +++ b/Source/cmAddCustomCommandCommand.cxx @@ -3,6 +3,7 @@ #include "cmAddCustomCommandCommand.h" #include +#include #include #include "cmCustomCommand.h" @@ -69,57 +70,106 @@ bool cmAddCustomCommandCommand::InitialPass( tdoing doing = doing_nothing; +#define MAKE_STATIC_KEYWORD(KEYWORD) \ + static const std::string key##KEYWORD = #KEYWORD + MAKE_STATIC_KEYWORD(APPEND); + MAKE_STATIC_KEYWORD(ARGS); + MAKE_STATIC_KEYWORD(BYPRODUCTS); + MAKE_STATIC_KEYWORD(COMMAND); + MAKE_STATIC_KEYWORD(COMMAND_EXPAND_LISTS); + MAKE_STATIC_KEYWORD(COMMENT); + MAKE_STATIC_KEYWORD(DEPENDS); + MAKE_STATIC_KEYWORD(DEPFILE); + MAKE_STATIC_KEYWORD(IMPLICIT_DEPENDS); + MAKE_STATIC_KEYWORD(MAIN_DEPENDENCY); + MAKE_STATIC_KEYWORD(OUTPUT); + MAKE_STATIC_KEYWORD(OUTPUTS); + MAKE_STATIC_KEYWORD(POST_BUILD); + MAKE_STATIC_KEYWORD(PRE_BUILD); + MAKE_STATIC_KEYWORD(PRE_LINK); + MAKE_STATIC_KEYWORD(SOURCE); + MAKE_STATIC_KEYWORD(TARGET); + MAKE_STATIC_KEYWORD(USES_TERMINAL); + MAKE_STATIC_KEYWORD(VERBATIM); + MAKE_STATIC_KEYWORD(WORKING_DIRECTORY); +#undef MAKE_STATIC_KEYWORD + static std::unordered_set keywords; + if (keywords.empty()) { + keywords.insert(keyAPPEND); + keywords.insert(keyARGS); + keywords.insert(keyBYPRODUCTS); + keywords.insert(keyCOMMAND); + keywords.insert(keyCOMMAND_EXPAND_LISTS); + keywords.insert(keyCOMMENT); + keywords.insert(keyDEPENDS); + keywords.insert(keyDEPFILE); + keywords.insert(keyIMPLICIT_DEPENDS); + keywords.insert(keyMAIN_DEPENDENCY); + keywords.insert(keyOUTPUT); + keywords.insert(keyOUTPUTS); + keywords.insert(keyPOST_BUILD); + keywords.insert(keyPRE_BUILD); + keywords.insert(keyPRE_LINK); + keywords.insert(keySOURCE); + keywords.insert(keyTARGET); + keywords.insert(keyUSES_TERMINAL); + keywords.insert(keyVERBATIM); + keywords.insert(keyWORKING_DIRECTORY); + } + for (std::string const& copy : args) { - if (copy == "SOURCE") { - doing = doing_source; - } else if (copy == "COMMAND") { - doing = doing_command; + if (keywords.count(copy)) { + if (copy == keySOURCE) { + doing = doing_source; + } else if (copy == keyCOMMAND) { + doing = doing_command; - // Save the current command before starting the next command. - if (!currentLine.empty()) { - commandLines.push_back(currentLine); - currentLine.clear(); - } - } else if (copy == "PRE_BUILD") { - cctype = cmTarget::PRE_BUILD; - } else if (copy == "PRE_LINK") { - cctype = cmTarget::PRE_LINK; - } else if (copy == "POST_BUILD") { - cctype = cmTarget::POST_BUILD; - } else if (copy == "VERBATIM") { - verbatim = true; - } else if (copy == "APPEND") { - append = true; - } else if (copy == "USES_TERMINAL") { - uses_terminal = true; - } else if (copy == "COMMAND_EXPAND_LISTS") { - command_expand_lists = true; - } else if (copy == "TARGET") { - doing = doing_target; - } else if (copy == "ARGS") { - // Ignore this old keyword. - } else if (copy == "DEPENDS") { - doing = doing_depends; - } else if (copy == "OUTPUTS") { - doing = doing_outputs; - } else if (copy == "OUTPUT") { - doing = doing_output; - } else if (copy == "BYPRODUCTS") { - doing = doing_byproducts; - } else if (copy == "WORKING_DIRECTORY") { - doing = doing_working_directory; - } else if (copy == "MAIN_DEPENDENCY") { - doing = doing_main_dependency; - } else if (copy == "IMPLICIT_DEPENDS") { - doing = doing_implicit_depends_lang; - } else if (copy == "COMMENT") { - doing = doing_comment; - } else if (copy == "DEPFILE") { - doing = doing_depfile; - if (this->Makefile->GetGlobalGenerator()->GetName() != "Ninja") { - this->SetError("Option DEPFILE not supported by " + - this->Makefile->GetGlobalGenerator()->GetName()); - return false; + // Save the current command before starting the next command. + if (!currentLine.empty()) { + commandLines.push_back(currentLine); + currentLine.clear(); + } + } else if (copy == keyPRE_BUILD) { + cctype = cmTarget::PRE_BUILD; + } else if (copy == keyPRE_LINK) { + cctype = cmTarget::PRE_LINK; + } else if (copy == keyPOST_BUILD) { + cctype = cmTarget::POST_BUILD; + } else if (copy == keyVERBATIM) { + verbatim = true; + } else if (copy == keyAPPEND) { + append = true; + } else if (copy == keyUSES_TERMINAL) { + uses_terminal = true; + } else if (copy == keyCOMMAND_EXPAND_LISTS) { + command_expand_lists = true; + } else if (copy == keyTARGET) { + doing = doing_target; + } else if (copy == keyARGS) { + // Ignore this old keyword. + } else if (copy == keyDEPENDS) { + doing = doing_depends; + } else if (copy == keyOUTPUTS) { + doing = doing_outputs; + } else if (copy == keyOUTPUT) { + doing = doing_output; + } else if (copy == keyBYPRODUCTS) { + doing = doing_byproducts; + } else if (copy == keyWORKING_DIRECTORY) { + doing = doing_working_directory; + } else if (copy == keyMAIN_DEPENDENCY) { + doing = doing_main_dependency; + } else if (copy == keyIMPLICIT_DEPENDS) { + doing = doing_implicit_depends_lang; + } else if (copy == keyCOMMENT) { + doing = doing_comment; + } else if (copy == keyDEPFILE) { + doing = doing_depfile; + if (this->Makefile->GetGlobalGenerator()->GetName() != "Ninja") { + this->SetError("Option DEPFILE not supported by " + + this->Makefile->GetGlobalGenerator()->GetName()); + return false; + } } } else { std::string filename; -- cgit v0.12 From f9235fd474682ba95231de0bc7285a39e35db83f Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 23 Jan 2018 18:30:26 -0500 Subject: cmAddCustomCommandCommand: use std::string const& for FileIsFullPath --- Source/cmAddCustomCommandCommand.cxx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/cmAddCustomCommandCommand.cxx b/Source/cmAddCustomCommandCommand.cxx index 3186583..6db50fa 100644 --- a/Source/cmAddCustomCommandCommand.cxx +++ b/Source/cmAddCustomCommandCommand.cxx @@ -177,7 +177,7 @@ bool cmAddCustomCommandCommand::InitialPass( case doing_output: case doing_outputs: case doing_byproducts: - if (!cmSystemTools::FileIsFullPath(copy.c_str())) { + if (!cmSystemTools::FileIsFullPath(copy)) { // This is an output to be generated, so it should be // under the build tree. CMake 2.4 placed this under the // source tree. However the only case that this change -- cgit v0.12 From 6dfd0f929416dbdf50ca44ebe19418e8a45782ad Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 23 Jan 2018 18:30:44 -0500 Subject: cmGeneratorExpressionNode: avoid some strlen in $ --- Source/cmGeneratorExpressionNode.cxx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Source/cmGeneratorExpressionNode.cxx b/Source/cmGeneratorExpressionNode.cxx index 34ef45f..5e3e810 100644 --- a/Source/cmGeneratorExpressionNode.cxx +++ b/Source/cmGeneratorExpressionNode.cxx @@ -947,7 +947,8 @@ static const struct TargetPropertyNode : public cmGeneratorExpressionNode "Target name not supported."); return std::string(); } - if (propertyName == "ALIASED_TARGET") { + static const std::string propALIASED_TARGET = "ALIASED_TARGET"; + if (propertyName == propALIASED_TARGET) { if (context->LG->GetMakefile()->IsAlias(targetName)) { if (cmGeneratorTarget* tgt = context->LG->FindGeneratorTargetToUse(targetName)) { -- cgit v0.12 From f2b8d67f1946bc59e95d1184f7ad64ac16193466 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 23 Jan 2018 18:31:15 -0500 Subject: cmTarget: use static strings for special property names Similar to 660769151a7f628f92eb28d77bcae854eaae54c2, the `SetProperty` side is showing up in performance listings due to string comparisons. --- Source/cmTarget.cxx | 42 ++++++++++++++++++++++++++++-------------- 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 33437a1..cd11c4b 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -861,39 +861,53 @@ void cmTarget::SetProperty(const std::string& prop, const char* value) this->Makefile->GetBacktrace())) { return; } - if (prop == "MANUALLY_ADDED_DEPENDENCIES") { +#define MAKE_STATIC_PROP(PROP) static const std::string prop##PROP = #PROP + MAKE_STATIC_PROP(COMPILE_DEFINITIONS); + MAKE_STATIC_PROP(COMPILE_FEATURES); + MAKE_STATIC_PROP(COMPILE_OPTIONS); + MAKE_STATIC_PROP(CUDA_PTX_COMPILATION); + MAKE_STATIC_PROP(EXPORT_NAME); + MAKE_STATIC_PROP(IMPORTED_GLOBAL); + MAKE_STATIC_PROP(INCLUDE_DIRECTORIES); + MAKE_STATIC_PROP(LINK_LIBRARIES); + MAKE_STATIC_PROP(MANUALLY_ADDED_DEPENDENCIES); + MAKE_STATIC_PROP(NAME); + MAKE_STATIC_PROP(SOURCES); + MAKE_STATIC_PROP(TYPE); +#undef MAKE_STATIC_PROP + if (prop == propMANUALLY_ADDED_DEPENDENCIES) { std::ostringstream e; e << "MANUALLY_ADDED_DEPENDENCIES property is read-only\n"; this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); return; } - if (prop == "NAME") { + if (prop == propNAME) { std::ostringstream e; e << "NAME property is read-only\n"; this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); return; } - if (prop == "TYPE") { + if (prop == propTYPE) { std::ostringstream e; e << "TYPE property is read-only\n"; this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); return; } - if (prop == "EXPORT_NAME" && this->IsImported()) { + if (prop == propEXPORT_NAME && this->IsImported()) { std::ostringstream e; e << "EXPORT_NAME property can't be set on imported targets (\"" << this->Name << "\")\n"; this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); return; } - if (prop == "SOURCES" && this->IsImported()) { + if (prop == propSOURCES && this->IsImported()) { std::ostringstream e; e << "SOURCES property can't be set on imported targets (\"" << this->Name << "\")\n"; this->Makefile->IssueMessage(cmake::FATAL_ERROR, e.str()); return; } - if (prop == "IMPORTED_GLOBAL" && !this->IsImported()) { + if (prop == propIMPORTED_GLOBAL && !this->IsImported()) { std::ostringstream e; e << "IMPORTED_GLOBAL property can't be set on non-imported targets (\"" << this->Name << "\")\n"; @@ -901,7 +915,7 @@ void cmTarget::SetProperty(const std::string& prop, const char* value) return; } - if (prop == "INCLUDE_DIRECTORIES") { + if (prop == propINCLUDE_DIRECTORIES) { this->Internal->IncludeDirectoriesEntries.clear(); this->Internal->IncludeDirectoriesBacktraces.clear(); if (value) { @@ -909,7 +923,7 @@ void cmTarget::SetProperty(const std::string& prop, const char* value) cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); this->Internal->IncludeDirectoriesBacktraces.push_back(lfbt); } - } else if (prop == "COMPILE_OPTIONS") { + } else if (prop == propCOMPILE_OPTIONS) { this->Internal->CompileOptionsEntries.clear(); this->Internal->CompileOptionsBacktraces.clear(); if (value) { @@ -917,7 +931,7 @@ void cmTarget::SetProperty(const std::string& prop, const char* value) cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); this->Internal->CompileOptionsBacktraces.push_back(lfbt); } - } else if (prop == "COMPILE_FEATURES") { + } else if (prop == propCOMPILE_FEATURES) { this->Internal->CompileFeaturesEntries.clear(); this->Internal->CompileFeaturesBacktraces.clear(); if (value) { @@ -925,7 +939,7 @@ void cmTarget::SetProperty(const std::string& prop, const char* value) cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); this->Internal->CompileFeaturesBacktraces.push_back(lfbt); } - } else if (prop == "COMPILE_DEFINITIONS") { + } else if (prop == propCOMPILE_DEFINITIONS) { this->Internal->CompileDefinitionsEntries.clear(); this->Internal->CompileDefinitionsBacktraces.clear(); if (value) { @@ -933,7 +947,7 @@ void cmTarget::SetProperty(const std::string& prop, const char* value) cmListFileBacktrace lfbt = this->Makefile->GetBacktrace(); this->Internal->CompileDefinitionsBacktraces.push_back(lfbt); } - } else if (prop == "LINK_LIBRARIES") { + } else if (prop == propLINK_LIBRARIES) { this->Internal->LinkImplementationPropertyEntries.clear(); this->Internal->LinkImplementationPropertyBacktraces.clear(); if (value) { @@ -941,7 +955,7 @@ void cmTarget::SetProperty(const std::string& prop, const char* value) this->Internal->LinkImplementationPropertyEntries.push_back(value); this->Internal->LinkImplementationPropertyBacktraces.push_back(lfbt); } - } else if (prop == "SOURCES") { + } else if (prop == propSOURCES) { this->Internal->SourceEntries.clear(); this->Internal->SourceBacktraces.clear(); if (value) { @@ -949,7 +963,7 @@ void cmTarget::SetProperty(const std::string& prop, const char* value) this->Internal->SourceEntries.push_back(value); this->Internal->SourceBacktraces.push_back(lfbt); } - } else if (prop == "IMPORTED_GLOBAL") { + } else if (prop == propIMPORTED_GLOBAL) { if (!cmSystemTools::IsOn(value)) { std::ostringstream e; e << "IMPORTED_GLOBAL property can't be set to FALSE on targets (\"" @@ -965,7 +979,7 @@ void cmTarget::SetProperty(const std::string& prop, const char* value) } else if (cmHasLiteralPrefix(prop, "IMPORTED_LIBNAME") && !this->CheckImportedLibName(prop, value ? value : "")) { /* error was reported by check method */ - } else if (prop == "CUDA_PTX_COMPILATION" && + } else if (prop == propCUDA_PTX_COMPILATION && this->GetType() != cmStateEnums::OBJECT_LIBRARY) { std::ostringstream e; e << "CUDA_PTX_COMPILATION property can only be applied to OBJECT " -- cgit v0.12 From 14a13d30eefd657e56022d30d8976cfdfaf9ab06 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Tue, 23 Jan 2018 19:31:17 -0500 Subject: cmGeneratorExpressionLexer: only tokenize strings with a '$' In standard libraries, `std::string::find` is usually implemented using vectorized code. Since the Tokenize method iterates character-by-character, doing an initial check using `find` improves performance. --- Source/cmGeneratorExpressionLexer.cxx | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Source/cmGeneratorExpressionLexer.cxx b/Source/cmGeneratorExpressionLexer.cxx index 95c79c1..e37f165 100644 --- a/Source/cmGeneratorExpressionLexer.cxx +++ b/Source/cmGeneratorExpressionLexer.cxx @@ -21,6 +21,12 @@ std::vector cmGeneratorExpressionLexer::Tokenize( { std::vector result; + if (input.find('$') == std::string::npos) { + result.push_back(cmGeneratorExpressionToken( + cmGeneratorExpressionToken::Text, input.c_str(), input.size())); + return result; + } + const char* c = input.c_str(); const char* upto = c; -- cgit v0.12 From 88ed556d99068a5e1bf303e69a156a25fb985bcf Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 24 Jan 2018 07:58:21 -0500 Subject: cmGeneratorTarget: make keyword strings const --- Source/cmGeneratorTarget.cxx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Source/cmGeneratorTarget.cxx b/Source/cmGeneratorTarget.cxx index ef3dc10..449b0c5 100644 --- a/Source/cmGeneratorTarget.cxx +++ b/Source/cmGeneratorTarget.cxx @@ -3616,13 +3616,13 @@ void cmGeneratorTarget::CheckPropertyCompatibility( const cmComputeLinkInformation::ItemVector& deps = info->GetItems(); std::set emittedBools; - static std::string strBool = "COMPATIBLE_INTERFACE_BOOL"; + static const std::string strBool = "COMPATIBLE_INTERFACE_BOOL"; std::set emittedStrings; - static std::string strString = "COMPATIBLE_INTERFACE_STRING"; + static const std::string strString = "COMPATIBLE_INTERFACE_STRING"; std::set emittedMinNumbers; - static std::string strNumMin = "COMPATIBLE_INTERFACE_NUMBER_MIN"; + static const std::string strNumMin = "COMPATIBLE_INTERFACE_NUMBER_MIN"; std::set emittedMaxNumbers; - static std::string strNumMax = "COMPATIBLE_INTERFACE_NUMBER_MAX"; + static const std::string strNumMax = "COMPATIBLE_INTERFACE_NUMBER_MAX"; for (auto const& dep : deps) { if (!dep.Target) { -- cgit v0.12 From 901c4a1e05ae7d01dcf4208dfd1b11e1534d4895 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 24 Jan 2018 07:57:57 -0500 Subject: cmExpandedCommandArgument: add an overload for const char* Static string comparisons were causing heap allocations just for a comparison. --- Source/cmExpandedCommandArgument.cxx | 5 +++++ Source/cmExpandedCommandArgument.h | 1 + 2 files changed, 6 insertions(+) diff --git a/Source/cmExpandedCommandArgument.cxx b/Source/cmExpandedCommandArgument.cxx index 0bea65f..1c0a721 100644 --- a/Source/cmExpandedCommandArgument.cxx +++ b/Source/cmExpandedCommandArgument.cxx @@ -24,6 +24,11 @@ bool cmExpandedCommandArgument::WasQuoted() const return this->Quoted; } +bool cmExpandedCommandArgument::operator==(const char* value) const +{ + return this->Value == value; +} + bool cmExpandedCommandArgument::operator==(std::string const& value) const { return this->Value == value; diff --git a/Source/cmExpandedCommandArgument.h b/Source/cmExpandedCommandArgument.h index fe86528..302e8db 100644 --- a/Source/cmExpandedCommandArgument.h +++ b/Source/cmExpandedCommandArgument.h @@ -24,6 +24,7 @@ public: bool WasQuoted() const; + bool operator==(const char* value) const; bool operator==(std::string const& value) const; bool empty() const; -- cgit v0.12