From c9c397a14ac59b25e43ce43264a420b2f9537547 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 29 Jul 2019 10:19:32 -0400 Subject: fileapi: Avoid unnecessary CompileData move --- Source/cmFileAPICodemodel.cxx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Source/cmFileAPICodemodel.cxx b/Source/cmFileAPICodemodel.cxx index 7b916cd..0c4f5c4 100644 --- a/Source/cmFileAPICodemodel.cxx +++ b/Source/cmFileAPICodemodel.cxx @@ -287,7 +287,7 @@ class Target Json::ArrayIndex si); void AddBacktrace(Json::Value& object, cmListFileBacktrace const& bt); Json::Value DumpPaths(); - Json::Value DumpCompileData(CompileData cd); + Json::Value DumpCompileData(CompileData const& cd); Json::Value DumpInclude(CompileData::IncludeEntry const& inc); Json::Value DumpDefine(BT const& def); Json::Value DumpSources(); @@ -915,7 +915,7 @@ Json::Value Target::DumpSource(cmGeneratorTarget::SourceAndKind const& sk, return source; } -Json::Value Target::DumpCompileData(CompileData cd) +Json::Value Target::DumpCompileData(CompileData const& cd) { Json::Value result = Json::objectValue; -- cgit v0.12 From 833d9eae4e49ef7b9055c709fd8a6959588952c3 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 29 Jul 2019 10:06:18 -0400 Subject: fileapi: Refactor codemodel defines de-duplication --- Source/cmFileAPICodemodel.cxx | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/Source/cmFileAPICodemodel.cxx b/Source/cmFileAPICodemodel.cxx index 0c4f5c4..9ff8ffa 100644 --- a/Source/cmFileAPICodemodel.cxx +++ b/Source/cmFileAPICodemodel.cxx @@ -231,8 +231,6 @@ struct CompileData } }; - void SetDefines(std::set> const& defines); - std::string Language; std::string Sysroot; std::vector> Flags; @@ -240,14 +238,6 @@ struct CompileData std::vector Includes; }; -void CompileData::SetDefines(std::set> const& defines) -{ - this->Defines.reserve(defines.size()); - for (BT const& d : defines) { - this->Defines.push_back(d); - } -} - class Target { cmGeneratorTarget* GT; @@ -726,7 +716,10 @@ void Target::ProcessLanguage(std::string const& lang) } std::set> defines = lg->GetTargetDefines(this->GT, this->Config, lang); - cd.SetDefines(defines); + cd.Defines.reserve(defines.size()); + for (BT const& d : defines) { + cd.Defines.emplace_back(d); + } std::vector> includePathList = lg->GetIncludeDirectories(this->GT, lang, this->Config); for (BT const& i : includePathList) { @@ -814,11 +807,16 @@ CompileData Target::BuildCompileData(cmSourceFile* sf) genexInterpreter.Evaluate(config_defs, COMPILE_DEFINITIONS)); } - std::set> defines; - defines.insert(fileDefines.begin(), fileDefines.end()); - defines.insert(cd.Defines.begin(), cd.Defines.end()); + fd.Defines.reserve(cd.Defines.size() + fileDefines.size()); + fd.Defines = cd.Defines; + for (std::string const& d : fileDefines) { + fd.Defines.emplace_back(d, cmListFileBacktrace()); + } - fd.SetDefines(defines); + // De-duplicate defines. + std::stable_sort(fd.Defines.begin(), fd.Defines.end()); + auto end = std::unique(fd.Defines.begin(), fd.Defines.end()); + fd.Defines.erase(end, fd.Defines.end()); return fd; } -- cgit v0.12 From d89c0ecf79a791c0b5ffff9fbb59e8720ee88950 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 29 Jul 2019 10:10:16 -0400 Subject: fileapi: Generate codemodel Json backtraces earlier Convert from `cmListFileBacktrace` to Json `backtraceGraph` entries before storing in `CompileData`. This will allow backtraces to be uniquely identified, hashed, and compared as a single integer. --- Source/cmFileAPICodemodel.cxx | 112 +++++++++++++++++++++++++++++------------- 1 file changed, 79 insertions(+), 33 deletions(-) diff --git a/Source/cmFileAPICodemodel.cxx b/Source/cmFileAPICodemodel.cxx index 9ff8ffa..88ddf7e 100644 --- a/Source/cmFileAPICodemodel.cxx +++ b/Source/cmFileAPICodemodel.cxx @@ -135,6 +135,36 @@ std::string TargetId(cmGeneratorTarget const* gt, std::string const& topBuild) return gt->GetName() + CMAKE_DIRECTORY_ID_SEP + hash; } +class JBTIndex +{ +public: + JBTIndex() = default; + explicit operator bool() const { return Index != None; } + Json::ArrayIndex Index = None; + static Json::ArrayIndex const None = static_cast(-1); +}; + +template +class JBT +{ +public: + JBT(T v = T(), JBTIndex bt = JBTIndex()) + : Value(std::move(v)) + , Backtrace(bt) + { + } + T Value; + JBTIndex Backtrace; + static bool ValueEq(JBT const& l, JBT const& r) + { + return l.Value == r.Value; + } + static bool ValueLess(JBT const& l, JBT const& r) + { + return l.Value < r.Value; + } +}; + class BacktraceData { std::string TopSource; @@ -169,7 +199,7 @@ class BacktraceData public: BacktraceData(std::string topSource); - bool Add(cmListFileBacktrace const& bt, Json::ArrayIndex& index); + JBTIndex Add(cmListFileBacktrace const& bt); Json::Value Dump(); }; @@ -178,16 +208,17 @@ BacktraceData::BacktraceData(std::string topSource) { } -bool BacktraceData::Add(cmListFileBacktrace const& bt, Json::ArrayIndex& index) +JBTIndex BacktraceData::Add(cmListFileBacktrace const& bt) { + JBTIndex index; if (bt.Empty()) { - return false; + return index; } cmListFileContext const* top = &bt.Top(); auto found = this->NodeMap.find(top); if (found != this->NodeMap.end()) { - index = found->second; - return true; + index.Index = found->second; + return index; } Json::Value entry = Json::objectValue; entry["file"] = this->AddFile(top->FilePath); @@ -197,13 +228,12 @@ bool BacktraceData::Add(cmListFileBacktrace const& bt, Json::ArrayIndex& index) if (!top->Name.empty()) { entry["command"] = this->AddCommand(top->Name); } - Json::ArrayIndex parent; - if (this->Add(bt.Pop(), parent)) { - entry["parent"] = parent; + if (JBTIndex parent = this->Add(bt.Pop())) { + entry["parent"] = parent.Index; } - index = this->NodeMap[top] = this->Nodes.size(); + index.Index = this->NodeMap[top] = this->Nodes.size(); this->Nodes.append(std::move(entry)); // NOLINT(*) - return true; + return index; } Json::Value BacktraceData::Dump() @@ -222,9 +252,9 @@ struct CompileData { struct IncludeEntry { - BT Path; + JBT Path; bool IsSystem = false; - IncludeEntry(BT path, bool isSystem) + IncludeEntry(JBT path, bool isSystem) : Path(std::move(path)) , IsSystem(isSystem) { @@ -233,8 +263,8 @@ struct CompileData std::string Language; std::string Sysroot; - std::vector> Flags; - std::vector> Defines; + std::vector> Flags; + std::vector> Defines; std::vector Includes; }; @@ -268,6 +298,12 @@ class Target std::map CompileGroupMap; std::vector CompileGroups; + template + JBT ToJBT(BT const& bt) + { + return JBT(bt.Value, this->Backtraces.Add(bt.Backtrace)); + } + void ProcessLanguages(); void ProcessLanguage(std::string const& lang); @@ -276,10 +312,11 @@ class Target Json::ArrayIndex AddSourceCompileGroup(cmSourceFile* sf, Json::ArrayIndex si); void AddBacktrace(Json::Value& object, cmListFileBacktrace const& bt); + void AddBacktrace(Json::Value& object, JBTIndex bt); Json::Value DumpPaths(); Json::Value DumpCompileData(CompileData const& cd); Json::Value DumpInclude(CompileData::IncludeEntry const& inc); - Json::Value DumpDefine(BT const& def); + Json::Value DumpDefine(JBT const& def); Json::Value DumpSources(); Json::Value DumpSource(cmGeneratorTarget::SourceAndKind const& sk, Json::ArrayIndex si); @@ -296,8 +333,8 @@ class Target Json::Value DumpLink(); Json::Value DumpArchive(); Json::Value DumpLinkCommandFragments(); - Json::Value DumpCommandFragments(std::vector> const& frags); - Json::Value DumpCommandFragment(BT const& frag, + Json::Value DumpCommandFragments(std::vector> const& frags); + Json::Value DumpCommandFragment(JBT const& frag, std::string const& role = std::string()); Json::Value DumpDependencies(); Json::Value DumpDependency(cmTargetDepend const& td); @@ -712,19 +749,20 @@ void Target::ProcessLanguage(std::string const& lang) // which may need to be factored out. std::string flags; lg->GetTargetCompileFlags(this->GT, this->Config, lang, flags); - cd.Flags.emplace_back(std::move(flags), cmListFileBacktrace()); + cd.Flags.emplace_back(std::move(flags), JBTIndex()); } std::set> defines = lg->GetTargetDefines(this->GT, this->Config, lang); cd.Defines.reserve(defines.size()); for (BT const& d : defines) { - cd.Defines.emplace_back(d); + cd.Defines.emplace_back(this->ToJBT(d)); } std::vector> includePathList = lg->GetIncludeDirectories(this->GT, lang, this->Config); for (BT const& i : includePathList) { cd.Includes.emplace_back( - i, this->GT->IsSystemIncludeDirectory(i.Value, this->Config, lang)); + this->ToJBT(i), + this->GT->IsSystemIncludeDirectory(i.Value, this->Config, lang)); } } @@ -763,14 +801,14 @@ CompileData Target::BuildCompileData(cmSourceFile* sf) const std::string COMPILE_FLAGS("COMPILE_FLAGS"); if (const char* cflags = sf->GetProperty(COMPILE_FLAGS)) { std::string flags = genexInterpreter.Evaluate(cflags, COMPILE_FLAGS); - fd.Flags.emplace_back(std::move(flags), cmListFileBacktrace()); + fd.Flags.emplace_back(std::move(flags), JBTIndex()); } const std::string COMPILE_OPTIONS("COMPILE_OPTIONS"); if (const char* coptions = sf->GetProperty(COMPILE_OPTIONS)) { std::string flags; lg->AppendCompileOptions( flags, genexInterpreter.Evaluate(coptions, COMPILE_OPTIONS)); - fd.Flags.emplace_back(std::move(flags), cmListFileBacktrace()); + fd.Flags.emplace_back(std::move(flags), JBTIndex()); } // Add include directories from source file properties. @@ -810,12 +848,14 @@ CompileData Target::BuildCompileData(cmSourceFile* sf) fd.Defines.reserve(cd.Defines.size() + fileDefines.size()); fd.Defines = cd.Defines; for (std::string const& d : fileDefines) { - fd.Defines.emplace_back(d, cmListFileBacktrace()); + fd.Defines.emplace_back(d, JBTIndex()); } // De-duplicate defines. - std::stable_sort(fd.Defines.begin(), fd.Defines.end()); - auto end = std::unique(fd.Defines.begin(), fd.Defines.end()); + std::stable_sort(fd.Defines.begin(), fd.Defines.end(), + JBT::ValueLess); + auto end = std::unique(fd.Defines.begin(), fd.Defines.end(), + JBT::ValueEq); fd.Defines.erase(end, fd.Defines.end()); return fd; @@ -843,9 +883,15 @@ Json::ArrayIndex Target::AddSourceCompileGroup(cmSourceFile* sf, void Target::AddBacktrace(Json::Value& object, cmListFileBacktrace const& bt) { - Json::ArrayIndex backtrace; - if (this->Backtraces.Add(bt, backtrace)) { - object["backtrace"] = backtrace; + if (JBTIndex backtrace = this->Backtraces.Add(bt)) { + object["backtrace"] = backtrace.Index; + } +} + +void Target::AddBacktrace(Json::Value& object, JBTIndex bt) +{ + if (bt) { + object["backtrace"] = bt.Index; } } @@ -935,7 +981,7 @@ Json::Value Target::DumpCompileData(CompileData const& cd) } if (!cd.Defines.empty()) { Json::Value defines = Json::arrayValue; - for (BT const& d : cd.Defines) { + for (JBT const& d : cd.Defines) { defines.append(this->DumpDefine(d)); } result["defines"] = std::move(defines); @@ -955,7 +1001,7 @@ Json::Value Target::DumpInclude(CompileData::IncludeEntry const& inc) return include; } -Json::Value Target::DumpDefine(BT const& def) +Json::Value Target::DumpDefine(JBT const& def) { Json::Value define = Json::objectValue; define["define"] = def.Value; @@ -1188,16 +1234,16 @@ Json::Value Target::DumpLinkCommandFragments() } Json::Value Target::DumpCommandFragments( - std::vector> const& frags) + std::vector> const& frags) { Json::Value commandFragments = Json::arrayValue; - for (BT const& f : frags) { + for (JBT const& f : frags) { commandFragments.append(this->DumpCommandFragment(f)); } return commandFragments; } -Json::Value Target::DumpCommandFragment(BT const& frag, +Json::Value Target::DumpCommandFragment(JBT const& frag, std::string const& role) { Json::Value fragment = Json::objectValue; -- cgit v0.12 From e337e60a50f3de8bb04b91b1233ff60377a9c944 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 29 Jul 2019 10:25:27 -0400 Subject: fileapi: Compute codemodel compile groups before converting to Json Previously we converted the description of each source file into its compile group Json object and then used the Json object itself as a unique identifier for the group. When source files have large descriptions their Json objects make inefficient map keys requiring deep comparison operations. Instead use our internal `CompileData` structure as a map key. This enables use of a hash map. Issue: #19520 --- Source/cmFileAPICodemodel.cxx | 65 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 56 insertions(+), 9 deletions(-) diff --git a/Source/cmFileAPICodemodel.cxx b/Source/cmFileAPICodemodel.cxx index 88ddf7e..2a15fd7 100644 --- a/Source/cmFileAPICodemodel.cxx +++ b/Source/cmFileAPICodemodel.cxx @@ -30,6 +30,9 @@ #include #include +#include +#include +#include #include #include #include @@ -155,6 +158,10 @@ public: } T Value; JBTIndex Backtrace; + friend bool operator==(JBT const& l, JBT const& r) + { + return l.Value == r.Value && l.Backtrace.Index == r.Backtrace.Index; + } static bool ValueEq(JBT const& l, JBT const& r) { return l.Value == r.Value; @@ -259,6 +266,10 @@ struct CompileData , IsSystem(isSystem) { } + friend bool operator==(IncludeEntry const& l, IncludeEntry const& r) + { + return l.Path == r.Path && l.IsSystem == r.IsSystem; + } }; std::string Language; @@ -266,8 +277,47 @@ struct CompileData std::vector> Flags; std::vector> Defines; std::vector Includes; + + friend bool operator==(CompileData const& l, CompileData const& r) + { + return (l.Language == r.Language && l.Sysroot == r.Sysroot && + l.Flags == r.Flags && l.Defines == r.Defines && + l.Includes == r.Includes); + } }; +} +namespace std { + +template <> +struct hash +{ + std::size_t operator()(CompileData const& in) const + { + using std::hash; + size_t result = + hash()(in.Language) ^ hash()(in.Sysroot); + for (auto const& i : in.Includes) { + result = result ^ + (hash()(i.Path.Value) ^ + hash()(i.Path.Backtrace.Index) ^ + (i.IsSystem ? std::numeric_limits::max() : 0)); + } + for (auto const& i : in.Flags) { + result = result ^ hash()(i.Value) ^ + hash()(i.Backtrace.Index); + } + for (auto const& i : in.Defines) { + result = result ^ hash()(i.Value) ^ + hash()(i.Backtrace.Index); + } + return result; + } +}; + +} // namespace std + +namespace { class Target { cmGeneratorTarget* GT; @@ -292,10 +342,10 @@ class Target struct CompileGroup { - std::map::iterator Entry; + std::unordered_map::iterator Entry; Json::Value SourceIndexes = Json::arrayValue; }; - std::map CompileGroupMap; + std::unordered_map CompileGroupMap; std::vector CompileGroups; template @@ -864,15 +914,12 @@ CompileData Target::BuildCompileData(cmSourceFile* sf) Json::ArrayIndex Target::AddSourceCompileGroup(cmSourceFile* sf, Json::ArrayIndex si) { - Json::Value compileDataJson = - this->DumpCompileData(this->BuildCompileData(sf)); - std::map::iterator i = - this->CompileGroupMap.find(compileDataJson); + CompileData compileData = this->BuildCompileData(sf); + auto i = this->CompileGroupMap.find(compileData); if (i == this->CompileGroupMap.end()) { Json::ArrayIndex cgIndex = static_cast(this->CompileGroups.size()); - i = - this->CompileGroupMap.emplace(std::move(compileDataJson), cgIndex).first; + i = this->CompileGroupMap.emplace(std::move(compileData), cgIndex).first; CompileGroup g; g.Entry = i; this->CompileGroups.push_back(std::move(g)); @@ -1037,7 +1084,7 @@ Json::Value Target::DumpCompileGroups() Json::Value Target::DumpCompileGroup(CompileGroup& cg) { - Json::Value group = cg.Entry->first; + Json::Value group = this->DumpCompileData(cg.Entry->first); group["sourceIndexes"] = std::move(cg.SourceIndexes); return group; } -- cgit v0.12 From b61fcdc8bc0d4b7b5104195abcfe03154e35bf52 Mon Sep 17 00:00:00 2001 From: Brad King Date: Mon, 29 Jul 2019 10:58:47 -0400 Subject: fileapi: Compute codemodel compile groups without target-wide settings Previously we computed the entire description of each source file including all target-wide settings, and then computed compile groups using those complete descriptions. This is inefficient when target-wide settings are large because they are included in comparisons even though they are the same for every source. Instead compute source groups using only the source-specific settings, and then merge the target-wide settings into place only once per unique compile group. This is a slight behavior change in the case that a source-specific compile definition duplicates a target-wide definition. Previously that source would still be grouped with other sources which do not have the definition because they would all get it from the target. Now that source will be in its own compile group even though it ultimately compiles with the same settings as another group. This is acceptable because the source is specified by the project with source-specific settings already. Fixes: #19520 --- Source/cmFileAPICodemodel.cxx | 53 ++++++++++++++++++++++++++++++++----------- 1 file changed, 40 insertions(+), 13 deletions(-) diff --git a/Source/cmFileAPICodemodel.cxx b/Source/cmFileAPICodemodel.cxx index 2a15fd7..e9ee7f5 100644 --- a/Source/cmFileAPICodemodel.cxx +++ b/Source/cmFileAPICodemodel.cxx @@ -359,6 +359,7 @@ class Target Json::ArrayIndex AddSourceGroup(cmSourceGroup* sg, Json::ArrayIndex si); CompileData BuildCompileData(cmSourceFile* sf); + CompileData MergeCompileData(CompileData const& fd); Json::ArrayIndex AddSourceCompileGroup(cmSourceFile* sf, Json::ArrayIndex si); void AddBacktrace(Json::Value& object, cmListFileBacktrace const& bt); @@ -839,15 +840,11 @@ CompileData Target::BuildCompileData(cmSourceFile* sf) if (fd.Language.empty()) { return fd; } - CompileData const& cd = this->CompileDataMap.at(fd.Language); - - fd.Sysroot = cd.Sysroot; cmLocalGenerator* lg = this->GT->GetLocalGenerator(); cmGeneratorExpressionInterpreter genexInterpreter(lg, this->Config, this->GT, fd.Language); - fd.Flags = cd.Flags; const std::string COMPILE_FLAGS("COMPILE_FLAGS"); if (const char* cflags = sf->GetProperty(COMPILE_FLAGS)) { std::string flags = genexInterpreter.Evaluate(cflags, COMPILE_FLAGS); @@ -877,8 +874,6 @@ CompileData Target::BuildCompileData(cmSourceFile* sf) } } } - fd.Includes.insert(fd.Includes.end(), cd.Includes.begin(), - cd.Includes.end()); const std::string COMPILE_DEFINITIONS("COMPILE_DEFINITIONS"); std::set fileDefines; @@ -895,20 +890,51 @@ CompileData Target::BuildCompileData(cmSourceFile* sf) genexInterpreter.Evaluate(config_defs, COMPILE_DEFINITIONS)); } - fd.Defines.reserve(cd.Defines.size() + fileDefines.size()); - fd.Defines = cd.Defines; + fd.Defines.reserve(fileDefines.size()); for (std::string const& d : fileDefines) { fd.Defines.emplace_back(d, JBTIndex()); } + return fd; +} + +CompileData Target::MergeCompileData(CompileData const& fd) +{ + CompileData cd; + cd.Language = fd.Language; + if (cd.Language.empty()) { + return cd; + } + CompileData const& td = this->CompileDataMap.at(cd.Language); + + // All compile groups share the sysroot of the target. + cd.Sysroot = td.Sysroot; + + // Use target-wide flags followed by source-specific flags. + cd.Flags.reserve(td.Flags.size() + fd.Flags.size()); + cd.Flags.insert(cd.Flags.end(), td.Flags.begin(), td.Flags.end()); + cd.Flags.insert(cd.Flags.end(), fd.Flags.begin(), fd.Flags.end()); + + // Use source-specific includes followed by target-wide includes. + cd.Includes.reserve(fd.Includes.size() + td.Includes.size()); + cd.Includes.insert(cd.Includes.end(), fd.Includes.begin(), + fd.Includes.end()); + cd.Includes.insert(cd.Includes.end(), td.Includes.begin(), + td.Includes.end()); + + // Use target-wide defines followed by source-specific defines. + cd.Defines.reserve(td.Defines.size() + fd.Defines.size()); + cd.Defines.insert(cd.Defines.end(), td.Defines.begin(), td.Defines.end()); + cd.Defines.insert(cd.Defines.end(), fd.Defines.begin(), fd.Defines.end()); + // De-duplicate defines. - std::stable_sort(fd.Defines.begin(), fd.Defines.end(), + std::stable_sort(cd.Defines.begin(), cd.Defines.end(), JBT::ValueLess); - auto end = std::unique(fd.Defines.begin(), fd.Defines.end(), + auto end = std::unique(cd.Defines.begin(), cd.Defines.end(), JBT::ValueEq); - fd.Defines.erase(end, fd.Defines.end()); + cd.Defines.erase(end, cd.Defines.end()); - return fd; + return cd; } Json::ArrayIndex Target::AddSourceCompileGroup(cmSourceFile* sf, @@ -1084,7 +1110,8 @@ Json::Value Target::DumpCompileGroups() Json::Value Target::DumpCompileGroup(CompileGroup& cg) { - Json::Value group = this->DumpCompileData(cg.Entry->first); + Json::Value group = + this->DumpCompileData(this->MergeCompileData(cg.Entry->first)); group["sourceIndexes"] = std::move(cg.SourceIndexes); return group; } -- cgit v0.12