From 5abfde6cb8a1ae0b2825797eab6c2e9842eb7c49 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 12 Mar 2014 14:01:45 -0400 Subject: [PATCH 1/3] cmDefinitions: Don't store parent lookups When looking up scopes, it is faster to not store the lookup locally to keep the maps smaller and avoid extra allocations and rebalancing. --- Source/cmDefinitions.cxx | 22 +++++++++++++++++----- Source/cmDefinitions.h | 10 ++++++---- Source/cmMakefile.cxx | 2 +- 3 files changed, 24 insertions(+), 10 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index babf1c4e8..98becf80d 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -28,7 +28,7 @@ void cmDefinitions::Reset(cmDefinitions* parent) //---------------------------------------------------------------------------- cmDefinitions::Def const& -cmDefinitions::GetInternal(const std::string& key) +cmDefinitions::GetInternal(const std::string& key) const { MapType::const_iterator i = this->Map.find(key); if(i != this->Map.end()) @@ -37,9 +37,8 @@ cmDefinitions::GetInternal(const std::string& key) } else if(cmDefinitions* up = this->Up) { - // Query the parent scope and store the result locally. - Def def = up->GetInternal(key); - return this->Map.insert(MapType::value_type(key, def)).first->second; + // Query the parent scope. + return up->GetInternal(key); } return this->NoDef; } @@ -71,12 +70,25 @@ cmDefinitions::SetInternal(const std::string& key, Def const& def) } //---------------------------------------------------------------------------- -const char* cmDefinitions::Get(const std::string& key) +const char* cmDefinitions::Get(const std::string& key) const { Def const& def = this->GetInternal(key); return def.Exists? def.c_str() : 0; } +//---------------------------------------------------------------------------- +void cmDefinitions::Pull(const std::string& key) +{ + if (this->Up) + { + Def const& def = this->Up->GetInternal(key); + if (def.Exists) + { + this->SetInternal(key, def); + } + } +} + //---------------------------------------------------------------------------- const char* cmDefinitions::Set(const std::string& key, const char* value) { diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index d615fb0e6..ebe6fa5f3 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -33,9 +33,11 @@ public: /** Returns the parent scope, if any. */ cmDefinitions* GetParent() const { return this->Up; } - /** Get the value associated with a key; null if none. - Store the result locally if it came from a parent. */ - const char* Get(const std::string& key); + /** Get the value associated with a key; null if none. */ + const char* Get(const std::string& key) const; + + /** Pull a variable from the parent. */ + void Pull(const std::string& key); /** Set (or unset if null) a value associated with a key. */ const char* Set(const std::string& key, const char* value); @@ -73,7 +75,7 @@ private: MapType Map; // Internal query and update methods. - Def const& GetInternal(const std::string& key); + Def const& GetInternal(const std::string& key) const; Def const& SetInternal(const std::string& key, Def const& def); // Implementation of Closure() method. diff --git a/Source/cmMakefile.cxx b/Source/cmMakefile.cxx index 630957fe7..412c998ac 100644 --- a/Source/cmMakefile.cxx +++ b/Source/cmMakefile.cxx @@ -4422,7 +4422,7 @@ void cmMakefile::RaiseScope(const std::string& var, const char *varDef) if(cmDefinitions* up = cur.GetParent()) { // First localize the definition in the current scope. - cur.Get(var); + cur.Pull(var); // Now update the definition in the parent scope. up->Set(var, varDef); From 3b21705d534c16a6197f28db68ea81e2816bfec3 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 12 Mar 2014 14:03:41 -0400 Subject: [PATCH 2/3] cmDefinitions: Avoid a find-then-insert when setting variables Searching the map is not necessary. --- Source/cmDefinitions.cxx | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 98becf80d..650216379 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -50,16 +50,7 @@ cmDefinitions::SetInternal(const std::string& key, Def const& def) if(this->Up || def.Exists) { // In lower scopes we store keys, defined or not. - MapType::iterator i = this->Map.find(key); - if(i == this->Map.end()) - { - i = this->Map.insert(MapType::value_type(key, def)).first; - } - else - { - i->second = def; - } - return i->second; + return (this->Map[key] = def); } else { From e17a69bc744ce0ed36e41be36694ca0053330d78 Mon Sep 17 00:00:00 2001 From: Ben Boeckel Date: Wed, 12 Mar 2014 01:48:06 -0400 Subject: [PATCH 3/3] cmDefinitions: Use a hashmap for faster checks The hash map is much faster at checking that the map won't have what we're looking for so that we can just go to the parent scope instead. --- Source/cmDefinitions.cxx | 5 +++-- Source/cmDefinitions.h | 7 +++++++ 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/Source/cmDefinitions.cxx b/Source/cmDefinitions.cxx index 650216379..5515f3551 100644 --- a/Source/cmDefinitions.cxx +++ b/Source/cmDefinitions.cxx @@ -15,7 +15,8 @@ cmDefinitions::Def cmDefinitions::NoDef; //---------------------------------------------------------------------------- -cmDefinitions::cmDefinitions(cmDefinitions* parent): Up(parent) +cmDefinitions::cmDefinitions(cmDefinitions* parent) + : Up(parent) { } @@ -35,7 +36,7 @@ cmDefinitions::GetInternal(const std::string& key) const { return i->second; } - else if(cmDefinitions* up = this->Up) + if(cmDefinitions* up = this->Up) { // Query the parent scope. return up->GetInternal(key); diff --git a/Source/cmDefinitions.h b/Source/cmDefinitions.h index ebe6fa5f3..5209a8b20 100644 --- a/Source/cmDefinitions.h +++ b/Source/cmDefinitions.h @@ -13,6 +13,9 @@ #define cmDefinitions_h #include "cmStandardIncludes.h" +#if defined(CMAKE_BUILD_WITH_CMAKE) +#include "cmsys/hash_map.hxx" +#endif /** \class cmDefinitions * \brief Store a scope of variable definitions for CMake language. @@ -71,7 +74,11 @@ private: cmDefinitions* Up; // Local definitions, set or unset. +#if defined(CMAKE_BUILD_WITH_CMAKE) + typedef cmsys::hash_map MapType; +#else typedef std::map MapType; +#endif MapType Map; // Internal query and update methods.