From 2b09d9f346bd3220b059771a6da1bafb06ce0f5b Mon Sep 17 00:00:00 2001 From: Stephen Kelly Date: Sun, 17 May 2015 13:37:41 +0200 Subject: cmMakefile: Remove VarInitStack. In cmMakefile::PushScope, a copy of the closure of keys initialized in the parent scope is made. In PopScope, essentially the same copy is inserted back into the parent. That means a lot of duplication of strings and a lot of string comparisons. None of it is needed, because the cmDefinitions keys already provide a canonical representation of what is initialized. The removal of the separate container also makes the variable handling code more easy to reason about in general. Before this patch, configuring llvm uses 200 KiB for the VarInitStack. Overall peak memory consumption goes from 35.5 MiB to 35.1 MiB. --- Source/cmDefinitions.cxx | 14 ++++++++++++++ Source/cmDefinitions.h | 3 +++ Source/cmMakefile.cxx | 32 +++++++++++--------------------- 3 files changed, 28 insertions(+), 21 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 97a16ea..5b03bd4 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -54,6 +54,20 @@ void cmDefinitions::Raise(const std::string& key, cmDefinitions::GetInternal(key, begin, end, true); } +bool cmDefinitions::HasKey(const std::string& key, + StackConstIter begin, StackConstIter end) +{ + for (StackConstIter it = begin; it != end; ++it) + { + MapType::const_iterator i = it->Map.find(key); + if (i != it->Map.end()) + { + return true; + } + } + return false; +} + //---------------------------------------------------------------------------- void cmDefinitions::Set(const std::string& key, const char* value) { diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index 894ff7a..bd3d392 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -40,6 +40,9 @@ public: static void Raise(const std::string& key, StackIter begin, StackIter end); + static bool HasKey(const std::string& key, + StackConstIter begin, StackConstIter end); + /** Set (or unset if null) a value associated with a key. */ void Set(const std::string& key, const char* value); diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 645a433..bad4e6a 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -47,7 +47,6 @@ class cmMakefile::Internals { public: std::list VarStack; - std::stack > VarInitStack; std::stack > VarUsageStack; bool IsSourceFileTryCompile; @@ -69,6 +68,12 @@ public: this->VarStack.rend()); } + bool IsInitialized(std::string const& name) + { + return cmDefinitions::HasKey(name, this->VarStack.rbegin(), + this->VarStack.rend()); + } + void SetDefinition(std::string const& name, std::string const& value) { this->VarStack.back().Set(name, value.c_str()); @@ -137,7 +142,6 @@ cmMakefile::cmMakefile(cmLocalGenerator* localGenerator) StateSnapshot(localGenerator->GetStateSnapshot()) { this->Internal->PushDefinitions(); - this->Internal->VarInitStack.push(std::set()); this->Internal->VarUsageStack.push(std::set()); this->Internal->IsSourceFileTryCompile = false; @@ -1719,13 +1723,12 @@ void cmMakefile::AddDefinition(const std::string& name, const char* value) return; } - this->Internal->SetDefinition(name, value); if (this->VariableInitialized(name)) { this->LogUnused("changing definition", name); this->Internal->VarUsageStack.top().erase(name); } - this->Internal->VarInitStack.top().insert(name); + this->Internal->SetDefinition(name, value); #ifdef CMAKE_BUILD_WITH_CMAKE cmVariableWatch* vv = this->GetVariableWatch(); @@ -1794,13 +1797,12 @@ void cmMakefile::AddCacheDefinition(const std::string& name, const char* value, void cmMakefile::AddDefinition(const std::string& name, bool value) { - this->Internal->SetDefinition(name, value ? "ON" : "OFF"); if (this->VariableInitialized(name)) { this->LogUnused("changing definition", name); this->Internal->VarUsageStack.top().erase(name); } - this->Internal->VarInitStack.top().insert(name); + this->Internal->SetDefinition(name, value ? "ON" : "OFF"); #ifdef CMAKE_BUILD_WITH_CMAKE cmVariableWatch* vv = this->GetVariableWatch(); if ( vv ) @@ -1832,12 +1834,7 @@ void cmMakefile::MarkVariableAsUsed(const std::string& var) bool cmMakefile::VariableInitialized(const std::string& var) const { - if(this->Internal->VarInitStack.top().find(var) != - this->Internal->VarInitStack.top().end()) - { - return true; - } - return false; + return this->Internal->IsInitialized(var); } bool cmMakefile::VariableUsed(const std::string& var) const @@ -1891,13 +1888,12 @@ void cmMakefile::LogUnused(const char* reason, void cmMakefile::RemoveDefinition(const std::string& name) { - this->Internal->RemoveDefinition(name); if (this->VariableInitialized(name)) { this->LogUnused("unsetting", name); this->Internal->VarUsageStack.top().erase(name); } - this->Internal->VarInitStack.top().insert(name); + this->Internal->RemoveDefinition(name); #ifdef CMAKE_BUILD_WITH_CMAKE cmVariableWatch* vv = this->GetVariableWatch(); if ( vv ) @@ -4316,9 +4312,7 @@ std::string cmMakefile::FormatListFileStack() const void cmMakefile::PushScope() { this->Internal->PushDefinitions(); - const std::set& init = this->Internal->VarInitStack.top(); const std::set& usage = this->Internal->VarUsageStack.top(); - this->Internal->VarInitStack.push(init); this->Internal->VarUsageStack.push(usage); this->PushLoopBlockBarrier(); @@ -4336,7 +4330,6 @@ void cmMakefile::PopScope() this->PopLoopBlockBarrier(); - std::set init = this->Internal->VarInitStack.top(); std::set usage = this->Internal->VarUsageStack.top(); const std::vector& locals = this->Internal->LocalKeys(); // Remove initialization and usage information for variables in the local @@ -4344,7 +4337,6 @@ void cmMakefile::PopScope() std::vector::const_iterator it = locals.begin(); for (; it != locals.end(); ++it) { - init.erase(*it); if (!this->VariableUsed(*it)) { this->LogUnused("out of scope", *it); @@ -4356,10 +4348,8 @@ void cmMakefile::PopScope() } this->Internal->PopDefinitions(); - this->Internal->VarInitStack.pop(); this->Internal->VarUsageStack.pop(); - // Push initialization and usage up to the parent scope. - this->Internal->VarInitStack.top().insert(init.begin(), init.end()); + // Push usage up to the parent scope. this->Internal->VarUsageStack.top().insert(usage.begin(), usage.end()); } -- cgit v0.12