Ver código fonte

cmListFileCache: Make cmListFileFunction a shared pointer

Passing cmListFileFunction everywhere by-value involves big overhead.
Now cmListFileFunction stores std::shared_ptr to the underlying data.
Oleksandr Koval 5 anos atrás
pai
commit
e614528ad1

+ 4 - 8
Source/cmCMakeLanguageCommand.cxx

@@ -77,18 +77,14 @@ bool cmCMakeLanguageCommandCALL(std::vector<cmListFileArgument> const& args,
   cmMakefile& makefile = status.GetMakefile();
   cmListFileContext context = makefile.GetBacktrace().Top();
 
-  cmListFileFunction func;
-  func.Name = callCommand;
-  func.Line = context.Line;
+  std::vector<cmListFileArgument> funcArgs;
+  funcArgs.reserve(args.size() - startArg);
 
   // The rest of the arguments are passed to the function call above
   for (size_t i = startArg; i < args.size(); ++i) {
-    cmListFileArgument lfarg;
-    lfarg.Delim = args[i].Delim;
-    lfarg.Line = context.Line;
-    lfarg.Value = args[i].Value;
-    func.Arguments.emplace_back(lfarg);
+    funcArgs.emplace_back(args[i].Value, args[i].Delim, context.Line);
   }
+  cmListFileFunction func{ callCommand, context.Line, std::move(funcArgs) };
 
   if (defer) {
     if (defer->Id.empty()) {

+ 6 - 3
Source/cmCPluginAPI.cxx

@@ -419,12 +419,15 @@ int CCONV cmExecuteCommand(void* arg, const char* name, int numArgs,
                            const char** args)
 {
   cmMakefile* mf = static_cast<cmMakefile*>(arg);
-  cmListFileFunction lff;
-  lff.Name = name;
+
+  std::vector<cmListFileArgument> lffArgs;
+  lffArgs.reserve(numArgs);
   for (int i = 0; i < numArgs; ++i) {
     // Assume all arguments are quoted.
-    lff.Arguments.emplace_back(args[i], cmListFileArgument::Quoted, 0);
+    lffArgs.emplace_back(args[i], cmListFileArgument::Quoted, 0);
   }
+
+  cmListFileFunction lff{ name, 0, std::move(lffArgs) };
   cmExecutionStatus status(*mf);
   return mf->ExecuteCommand(lff, status);
 }

+ 1 - 1
Source/cmForEachCommand.cxx

@@ -90,7 +90,7 @@ bool cmForEachFunctionBlocker::ArgumentsMatch(cmListFileFunction const& lff,
                                               cmMakefile& mf) const
 {
   std::vector<std::string> expandedArguments;
-  mf.ExpandArguments(lff.Arguments, expandedArguments);
+  mf.ExpandArguments(lff.Arguments(), expandedArguments);
   return expandedArguments.empty() ||
     expandedArguments.front() == this->Args.front();
 }

+ 2 - 2
Source/cmFunctionBlocker.cxx

@@ -15,9 +15,9 @@
 bool cmFunctionBlocker::IsFunctionBlocked(const cmListFileFunction& lff,
                                           cmExecutionStatus& status)
 {
-  if (lff.Name.Lower == this->StartCommandName()) {
+  if (lff.LowerCaseName() == this->StartCommandName()) {
     this->ScopeDepth++;
-  } else if (lff.Name.Lower == this->EndCommandName()) {
+  } else if (lff.LowerCaseName() == this->EndCommandName()) {
     this->ScopeDepth--;
     if (this->ScopeDepth == 0U) {
       cmMakefile& mf = status.GetMakefile();

+ 1 - 1
Source/cmFunctionCommand.cxx

@@ -147,7 +147,7 @@ bool cmFunctionFunctionBlocker::ArgumentsMatch(cmListFileFunction const& lff,
                                                cmMakefile& mf) const
 {
   std::vector<std::string> expandedArguments;
-  mf.ExpandArguments(lff.Arguments, expandedArguments);
+  mf.ExpandArguments(lff.Arguments(), expandedArguments);
   return expandedArguments.empty() ||
     expandedArguments.front() == this->Args.front();
 }

+ 12 - 11
Source/cmIfCommand.cxx

@@ -54,7 +54,7 @@ public:
 bool cmIfFunctionBlocker::ArgumentsMatch(cmListFileFunction const& lff,
                                          cmMakefile&) const
 {
-  return lff.Arguments.empty() || lff.Arguments == this->Args;
+  return lff.Arguments().empty() || lff.Arguments() == this->Args;
 }
 
 bool cmIfFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
@@ -65,19 +65,19 @@ bool cmIfFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
   int scopeDepth = 0;
   for (cmListFileFunction const& func : functions) {
     // keep track of scope depth
-    if (func.Name.Lower == "if") {
+    if (func.LowerCaseName() == "if") {
       scopeDepth++;
     }
-    if (func.Name.Lower == "endif") {
+    if (func.LowerCaseName() == "endif") {
       scopeDepth--;
     }
     // watch for our state change
-    if (scopeDepth == 0 && func.Name.Lower == "else") {
+    if (scopeDepth == 0 && func.LowerCaseName() == "else") {
 
       if (this->ElseSeen) {
-        cmListFileBacktrace elseBT = mf.GetBacktrace().Push(
-          cmListFileContext{ func.Name.Original,
-                             this->GetStartingContext().FilePath, func.Line });
+        cmListFileBacktrace elseBT = mf.GetBacktrace().Push(cmListFileContext{
+          func.OriginalName(), this->GetStartingContext().FilePath,
+          func.Line() });
         mf.GetCMakeInstance()->IssueMessage(
           MessageType::FATAL_ERROR,
           "A duplicate ELSE command was found inside an IF block.", elseBT);
@@ -94,9 +94,10 @@ bool cmIfFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
       if (!this->IsBlocking && mf.GetCMakeInstance()->GetTrace()) {
         mf.PrintCommandTrace(func);
       }
-    } else if (scopeDepth == 0 && func.Name.Lower == "elseif") {
-      cmListFileBacktrace elseifBT = mf.GetBacktrace().Push(cmListFileContext{
-        func.Name.Original, this->GetStartingContext().FilePath, func.Line });
+    } else if (scopeDepth == 0 && func.LowerCaseName() == "elseif") {
+      cmListFileBacktrace elseifBT = mf.GetBacktrace().Push(
+        cmListFileContext{ func.OriginalName(),
+                           this->GetStartingContext().FilePath, func.Line() });
       if (this->ElseSeen) {
         mf.GetCMakeInstance()->IssueMessage(
           MessageType::FATAL_ERROR,
@@ -116,7 +117,7 @@ bool cmIfFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,
         std::string errorString;
 
         std::vector<cmExpandedCommandArgument> expandedArguments;
-        mf.ExpandArguments(func.Arguments, expandedArguments);
+        mf.ExpandArguments(func.Arguments(), expandedArguments);
 
         MessageType messType;
 

+ 9 - 14
Source/cmListFileCache.cxx

@@ -15,14 +15,6 @@
 #include "cmStringAlgorithms.h"
 #include "cmSystemTools.h"
 
-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 lfbt,
@@ -43,7 +35,9 @@ struct cmListFileParser
   cmMessenger* Messenger;
   const char* FileName;
   cmListFileLexer* Lexer;
-  cmListFileFunction Function;
+  std::string FunctionName;
+  long FunctionLine;
+  std::vector<cmListFileArgument> FunctionArguments;
   enum
   {
     SeparationOkay,
@@ -141,7 +135,9 @@ bool cmListFileParser::Parse()
       if (haveNewline) {
         haveNewline = false;
         if (this->ParseFunction(token->text, token->line)) {
-          this->ListFile->Functions.push_back(this->Function);
+          this->ListFile->Functions.emplace_back(
+            std::move(this->FunctionName), this->FunctionLine,
+            std::move(this->FunctionArguments));
         } else {
           return false;
         }
@@ -200,9 +196,8 @@ bool cmListFile::ParseString(const char* str, const char* virtual_filename,
 bool cmListFileParser::ParseFunction(const char* name, long line)
 {
   // Ininitialize a new function call.
-  this->Function = cmListFileFunction();
-  this->Function.Name = name;
-  this->Function.Line = line;
+  this->FunctionName = name;
+  this->FunctionLine = line;
 
   // Command name has already been parsed.  Read the left paren.
   cmListFileLexer_Token* token;
@@ -297,7 +292,7 @@ bool cmListFileParser::ParseFunction(const char* name, long line)
 bool cmListFileParser::AddArgument(cmListFileLexer_Token* token,
                                    cmListFileArgument::Delimiter delim)
 {
-  this->Function.Arguments.emplace_back(token->text, delim, token->line);
+  this->FunctionArguments.emplace_back(token->text, delim, token->line);
   if (this->Separation == SeparationOkay) {
     return true;
   }

+ 50 - 7
Source/cmListFileCache.h

@@ -14,6 +14,7 @@
 #include <cm/optional>
 
 #include "cmStateSnapshot.h"
+#include "cmSystemTools.h"
 
 /** \class cmListFileCache
  * \brief A class to cache list file contents.
@@ -28,16 +29,19 @@ struct cmCommandContext
 {
   struct cmCommandName
   {
-    std::string Lower;
     std::string Original;
+    std::string Lower;
     cmCommandName() = default;
-    cmCommandName(std::string const& name) { *this = name; }
-    cmCommandName& operator=(std::string const& name);
+    cmCommandName(std::string name)
+      : Original(std::move(name))
+      , Lower(cmSystemTools::LowerCase(this->Original))
+    {
+    }
   } Name;
   long Line = 0;
   cmCommandContext() = default;
-  cmCommandContext(const char* name, int line)
-    : Name(name)
+  cmCommandContext(std::string name, long line)
+    : Name(std::move(name))
     , Line(line)
   {
   }
@@ -103,9 +107,48 @@ bool operator<(const cmListFileContext& lhs, const cmListFileContext& rhs);
 bool operator==(cmListFileContext const& lhs, cmListFileContext const& rhs);
 bool operator!=(cmListFileContext const& lhs, cmListFileContext const& rhs);
 
-struct cmListFileFunction : public cmCommandContext
+class cmListFileFunction
 {
-  std::vector<cmListFileArgument> Arguments;
+public:
+  cmListFileFunction(std::string name, long line,
+                     std::vector<cmListFileArgument> args)
+    : Impl{ std::make_shared<Implementation>(std::move(name), line,
+                                             std::move(args)) }
+  {
+  }
+
+  std::string const& OriginalName() const noexcept
+  {
+    return this->Impl->Name.Original;
+  }
+
+  std::string const& LowerCaseName() const noexcept
+  {
+    return this->Impl->Name.Lower;
+  }
+
+  long Line() const noexcept { return this->Impl->Line; }
+
+  std::vector<cmListFileArgument> const& Arguments() const noexcept
+  {
+    return this->Impl->Arguments;
+  }
+
+  operator cmCommandContext const&() const noexcept { return *this->Impl; }
+
+private:
+  struct Implementation : public cmCommandContext
+  {
+    Implementation(std::string name, long line,
+                   std::vector<cmListFileArgument> args)
+      : cmCommandContext{ std::move(name), line }
+      , Arguments{ std::move(args) }
+    {
+    }
+    std::vector<cmListFileArgument> Arguments;
+  };
+
+  std::shared_ptr<Implementation const> Impl;
 };
 
 // Represent a backtrace (call stack).  Provide value semantics

+ 7 - 8
Source/cmMacroCommand.cxx

@@ -81,17 +81,14 @@ bool cmMacroHelperCommand::operator()(
     argVs.emplace_back(argvName);
   }
   // Invoke all the functions that were collected in the block.
-  cmListFileFunction newLFF;
   // for each function
   for (cmListFileFunction const& func : this->Functions) {
     // Replace the formal arguments and then invoke the command.
-    newLFF.Arguments.clear();
-    newLFF.Arguments.reserve(func.Arguments.size());
-    newLFF.Name = func.Name;
-    newLFF.Line = func.Line;
+    std::vector<cmListFileArgument> newLFFArgs;
+    newLFFArgs.reserve(func.Arguments().size());
 
     // for each argument of the current function
-    for (cmListFileArgument const& k : func.Arguments) {
+    for (cmListFileArgument const& k : func.Arguments()) {
       cmListFileArgument arg;
       arg.Value = k.Value;
       if (k.Delim != cmListFileArgument::Bracket) {
@@ -116,8 +113,10 @@ bool cmMacroHelperCommand::operator()(
       }
       arg.Delim = k.Delim;
       arg.Line = k.Line;
-      newLFF.Arguments.push_back(std::move(arg));
+      newLFFArgs.push_back(std::move(arg));
     }
+    cmListFileFunction newLFF{ func.OriginalName(), func.Line(),
+                               std::move(newLFFArgs) };
     cmExecutionStatus status(makefile);
     if (!makefile.ExecuteCommand(newLFF, status) || status.GetNestedError()) {
       // The error message should have already included the call stack
@@ -157,7 +156,7 @@ bool cmMacroFunctionBlocker::ArgumentsMatch(cmListFileFunction const& lff,
                                             cmMakefile& mf) const
 {
   std::vector<std::string> expandedArguments;
-  mf.ExpandArguments(lff.Arguments, expandedArguments);
+  mf.ExpandArguments(lff.Arguments(), expandedArguments);
   return expandedArguments.empty() || expandedArguments[0] == this->Args[0];
 }
 

+ 21 - 21
Source/cmMakefile.cxx

@@ -306,8 +306,8 @@ void cmMakefile::PrintCommandTrace(
   std::string temp;
   bool expand = this->GetCMakeInstance()->GetTraceExpand();
 
-  args.reserve(lff.Arguments.size());
-  for (cmListFileArgument const& arg : lff.Arguments) {
+  args.reserve(lff.Arguments().size());
+  for (cmListFileArgument const& arg : lff.Arguments()) {
     if (expand) {
       temp = arg.Value;
       this->ExpandVariablesInString(temp);
@@ -324,11 +324,11 @@ void cmMakefile::PrintCommandTrace(
       Json::StreamWriterBuilder builder;
       builder["indentation"] = "";
       val["file"] = full_path;
-      val["line"] = static_cast<Json::Value::Int64>(lff.Line);
+      val["line"] = static_cast<Json::Value::Int64>(lff.Line());
       if (deferId) {
         val["defer"] = *deferId;
       }
-      val["cmd"] = lff.Name.Original;
+      val["cmd"] = lff.OriginalName();
       val["args"] = Json::Value(Json::arrayValue);
       for (std::string const& arg : args) {
         val["args"].append(arg);
@@ -341,11 +341,11 @@ void cmMakefile::PrintCommandTrace(
       break;
     }
     case cmake::TraceFormat::TRACE_HUMAN:
-      msg << full_path << "(" << lff.Line << "):";
+      msg << full_path << "(" << lff.Line() << "):";
       if (deferId) {
         msg << "DEFERRED:" << *deferId << ":";
       }
-      msg << "  " << lff.Name.Original << "(";
+      msg << "  " << lff.OriginalName() << "(";
 
       for (std::string const& arg : args) {
         msg << arg << " ";
@@ -451,7 +451,7 @@ bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff,
 
   // Lookup the command prototype.
   if (cmState::Command command =
-        this->GetState()->GetCommandByExactName(lff.Name.Lower)) {
+        this->GetState()->GetCommandByExactName(lff.LowerCaseName())) {
     // Decide whether to invoke the command.
     if (!cmSystemTools::GetFatalErrorOccured()) {
       // if trace is enabled, print out invoke information
@@ -459,13 +459,13 @@ bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff,
         this->PrintCommandTrace(lff, this->Backtrace.Top().DeferId);
       }
       // Try invoking the command.
-      bool invokeSucceeded = command(lff.Arguments, status);
+      bool invokeSucceeded = command(lff.Arguments(), status);
       bool hadNestedError = status.GetNestedError();
       if (!invokeSucceeded || hadNestedError) {
         if (!hadNestedError) {
           // The command invocation requested that we report an error.
           std::string const error =
-            std::string(lff.Name.Original) + " " + status.GetError();
+            std::string(lff.OriginalName()) + " " + status.GetError();
           this->IssueMessage(MessageType::FATAL_ERROR, error);
         }
         result = false;
@@ -477,7 +477,7 @@ bool cmMakefile::ExecuteCommand(const cmListFileFunction& lff,
   } else {
     if (!cmSystemTools::GetFatalErrorOccured()) {
       std::string error =
-        cmStrCat("Unknown CMake command \"", lff.Name.Original, "\".");
+        cmStrCat("Unknown CMake command \"", lff.OriginalName(), "\".");
       this->IssueMessage(MessageType::FATAL_ERROR, error);
       result = false;
       cmSystemTools::SetFatalErrorOccured();
@@ -1690,7 +1690,7 @@ void cmMakefile::Configure()
     bool hasVersion = false;
     // search for the right policy command
     for (cmListFileFunction const& func : listFile.Functions) {
-      if (func.Name.Lower == "cmake_minimum_required") {
+      if (func.LowerCaseName() == "cmake_minimum_required") {
         hasVersion = true;
         break;
       }
@@ -1717,7 +1717,7 @@ void cmMakefile::Configure()
         allowedCommands.insert("message");
         isProblem = false;
         for (cmListFileFunction const& func : listFile.Functions) {
-          if (!cm::contains(allowedCommands, func.Name.Lower)) {
+          if (!cm::contains(allowedCommands, func.LowerCaseName())) {
             isProblem = true;
             break;
           }
@@ -1737,7 +1737,7 @@ void cmMakefile::Configure()
     bool hasProject = false;
     // search for a project command
     for (cmListFileFunction const& func : listFile.Functions) {
-      if (func.Name.Lower == "project") {
+      if (func.LowerCaseName() == "project") {
         hasProject = true;
         break;
       }
@@ -1754,12 +1754,12 @@ void cmMakefile::Configure()
         "CMake is pretending there is a \"project(Project)\" command on "
         "the first line.",
         this->Backtrace);
-      cmListFileFunction project;
-      project.Name.Lower = "project";
-      project.Arguments.emplace_back("Project", cmListFileArgument::Unquoted,
-                                     0);
-      project.Arguments.emplace_back("__CMAKE_INJECTED_PROJECT_COMMAND__",
-                                     cmListFileArgument::Unquoted, 0);
+      cmListFileFunction project{ "project",
+                                  0,
+                                  { { "Project", cmListFileArgument::Unquoted,
+                                      0 },
+                                    { "__CMAKE_INJECTED_PROJECT_COMMAND__",
+                                      cmListFileArgument::Unquoted, 0 } } };
       listFile.Functions.insert(listFile.Functions.begin(), project);
     }
   }
@@ -3105,8 +3105,8 @@ cm::optional<std::string> cmMakefile::DeferGetCall(std::string const& id) const
     std::string tmp;
     for (DeferCommand const& dc : this->Defer->Commands) {
       if (dc.Id == id) {
-        tmp = dc.Command.Name.Original;
-        for (cmListFileArgument const& arg : dc.Command.Arguments) {
+        tmp = dc.Command.OriginalName();
+        for (cmListFileArgument const& arg : dc.Command.Arguments()) {
           tmp = cmStrCat(tmp, ';', arg.Value);
         }
         break;

+ 3 - 3
Source/cmMakefileProfilingData.cxx

@@ -58,7 +58,7 @@ void cmMakefileProfilingData::StartEntry(const cmListFileFunction& lff,
     cmsys::SystemInformation info;
     Json::Value v;
     v["ph"] = "B";
-    v["name"] = lff.Name.Lower;
+    v["name"] = lff.LowerCaseName();
     v["cat"] = "cmake";
     v["ts"] = Json::Value::UInt64(
       std::chrono::duration_cast<std::chrono::microseconds>(
@@ -67,9 +67,9 @@ void cmMakefileProfilingData::StartEntry(const cmListFileFunction& lff,
     v["pid"] = static_cast<int>(info.GetProcessId());
     v["tid"] = 0;
     Json::Value argsValue;
-    if (!lff.Arguments.empty()) {
+    if (!lff.Arguments().empty()) {
       std::string args;
-      for (const auto& a : lff.Arguments) {
+      for (auto const& a : lff.Arguments()) {
         args += (args.empty() ? "" : " ") + a.Value;
       }
       argsValue["functionArgs"] = args;

+ 1 - 1
Source/cmMakefileProfilingData.h

@@ -11,7 +11,7 @@ class StreamWriter;
 }
 
 class cmListFileContext;
-struct cmListFileFunction;
+class cmListFileFunction;
 
 class cmMakefileProfilingData
 {

+ 5 - 4
Source/cmVariableWatchCommand.cxx

@@ -45,20 +45,21 @@ void cmVariableWatchCommandVariableAccessed(const std::string& variable,
 
   std::string stack = *mf->GetProperty("LISTFILE_STACK");
   if (!data->Command.empty()) {
-    cmListFileFunction newLFF;
     cmProp const currentListFile =
       mf->GetDefinition("CMAKE_CURRENT_LIST_FILE");
     const auto fakeLineNo =
       std::numeric_limits<decltype(cmListFileArgument::Line)>::max();
-    newLFF.Arguments = {
+
+    std::vector<cmListFileArgument> newLFFArgs{
       { variable, cmListFileArgument::Quoted, fakeLineNo },
       { accessString, cmListFileArgument::Quoted, fakeLineNo },
       { newValue ? newValue : "", cmListFileArgument::Quoted, fakeLineNo },
       { *currentListFile, cmListFileArgument::Quoted, fakeLineNo },
       { stack, cmListFileArgument::Quoted, fakeLineNo }
     };
-    newLFF.Name = data->Command;
-    newLFF.Line = fakeLineNo;
+
+    cmListFileFunction newLFF{ data->Command, fakeLineNo,
+                               std::move(newLFFArgs) };
     cmExecutionStatus status(*makefile);
     if (!makefile->ExecuteCommand(newLFF, status)) {
       cmSystemTools::Error(

+ 1 - 1
Source/cmWhileCommand.cxx

@@ -54,7 +54,7 @@ cmWhileFunctionBlocker::~cmWhileFunctionBlocker()
 bool cmWhileFunctionBlocker::ArgumentsMatch(cmListFileFunction const& lff,
                                             cmMakefile&) const
 {
-  return lff.Arguments.empty() || lff.Arguments == this->Args;
+  return lff.Arguments().empty() || lff.Arguments() == this->Args;
 }
 
 bool cmWhileFunctionBlocker::Replay(std::vector<cmListFileFunction> functions,