From a4d58722a47b3712d1f78683371f88a3b6182a59 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 18 Jul 2014 13:57:57 -0400 Subject: [PATCH 1/2] cmTarget: Drop internal cache of link interface usage requirements These use a huge amount of memory that accumulates as generation proceeds. On the Unix Makefiles generator, only GetIncludeDirectories and GetCompileDefinitions are even called more than once per target (once for build files, once for dependency scanning preprocessor info). Another approach will be needed to avoid duplicate computation in the cases where it does occur. --- Source/cmTarget.cxx | 248 +++++++++++++++----------------------------- 1 file changed, 84 insertions(+), 164 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index 07f08deb9..a812df70f 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -213,23 +213,6 @@ public: void AddInterfaceEntries( cmTarget const* thisTarget, std::string const& config, std::string const& prop, std::vector& entries); - - std::map > - CachedLinkInterfaceIncludeDirectoriesEntries; - std::map > - CachedLinkInterfaceCompileOptionsEntries; - std::map > - CachedLinkInterfaceCompileDefinitionsEntries; - std::map > - CachedLinkInterfaceSourcesEntries; - std::map > - CachedLinkInterfaceCompileFeaturesEntries; - - std::map CacheLinkInterfaceIncludeDirectoriesDone; - std::map CacheLinkInterfaceCompileDefinitionsDone; - std::map CacheLinkInterfaceCompileOptionsDone; - std::map CacheLinkInterfaceSourcesDone; - std::map CacheLinkInterfaceCompileFeaturesDone; }; cmLinkImplItem cmTargetInternals::TargetPropertyEntry::NoLinkImplItem; @@ -248,27 +231,9 @@ static void deleteAndClear( entries.clear(); } -//---------------------------------------------------------------------------- -static void deleteAndClear( - std::map > &entries) -{ - for (std::map >::iterator - it = entries.begin(), end = entries.end(); it != end; ++it) - { - deleteAndClear(it->second); - } -} - //---------------------------------------------------------------------------- cmTargetInternals::~cmTargetInternals() { - deleteAndClear(this->CachedLinkInterfaceIncludeDirectoriesEntries); - deleteAndClear(this->CachedLinkInterfaceCompileOptionsEntries); - deleteAndClear(this->CachedLinkInterfaceCompileFeaturesEntries); - deleteAndClear(this->CachedLinkInterfaceCompileDefinitionsEntries); - deleteAndClear(this->CachedLinkInterfaceSourcesEntries); } //---------------------------------------------------------------------------- @@ -810,16 +775,16 @@ void cmTarget::GetSourceFiles(std::vector &files, config, debugSources); - if (!this->Internal->CacheLinkInterfaceSourcesDone[config]) - { - this->Internal->AddInterfaceEntries( - this, config, "INTERFACE_SOURCES", - this->Internal->CachedLinkInterfaceSourcesEntries[config]); - } + std::vector + linkInterfaceSourcesEntries; - std::vector::size_type numFilesBefore = files.size(); - bool contextDependentInterfaceSources = processSources(this, - this->Internal->CachedLinkInterfaceSourcesEntries[config], + this->Internal->AddInterfaceEntries( + this, config, "INTERFACE_SOURCES", + linkInterfaceSourcesEntries); + + std::vector::size_type numFilesBefore = files.size(); + bool contextDependentInterfaceSources = processSources(this, + linkInterfaceSourcesEntries, files, uniqueSrcs, &dagChecker, @@ -832,14 +797,7 @@ void cmTarget::GetSourceFiles(std::vector &files, this->LinkImplementationLanguageIsContextDependent = false; } - if (!this->Makefile->IsGeneratingBuildSystem()) - { - deleteAndClear(this->Internal->CachedLinkInterfaceSourcesEntries); - } - else - { - this->Internal->CacheLinkInterfaceSourcesDone[config] = true; - } + deleteAndClear(linkInterfaceSourcesEntries); } //---------------------------------------------------------------------------- @@ -2213,58 +2171,47 @@ cmTarget::GetIncludeDirectories(const std::string& config) const config, debugIncludes); - if (!this->Internal->CacheLinkInterfaceIncludeDirectoriesDone[config]) + std::vector + linkInterfaceIncludeDirectoriesEntries; + this->Internal->AddInterfaceEntries( + this, config, "INTERFACE_INCLUDE_DIRECTORIES", + linkInterfaceIncludeDirectoriesEntries); + + if(this->Makefile->IsOn("APPLE")) { - this->Internal->AddInterfaceEntries( - this, config, "INTERFACE_INCLUDE_DIRECTORIES", - this->Internal->CachedLinkInterfaceIncludeDirectoriesEntries[config]); - - if(this->Makefile->IsOn("APPLE")) + LinkImplementation const* impl = this->GetLinkImplementation(config); + for(std::vector::const_iterator + it = impl->Libraries.begin(); + it != impl->Libraries.end(); ++it) { - LinkImplementation const* impl = this->GetLinkImplementation(config); - for(std::vector::const_iterator - it = impl->Libraries.begin(); - it != impl->Libraries.end(); ++it) + std::string libDir = cmSystemTools::CollapseFullPath(it->c_str()); + + static cmsys::RegularExpression + frameworkCheck("(.*\\.framework)(/Versions/[^/]+)?/[^/]+$"); + if(!frameworkCheck.find(libDir)) { - std::string libDir = cmSystemTools::CollapseFullPath(it->c_str()); - - static cmsys::RegularExpression - frameworkCheck("(.*\\.framework)(/Versions/[^/]+)?/[^/]+$"); - if(!frameworkCheck.find(libDir)) - { - continue; - } - - libDir = frameworkCheck.match(1); - - cmGeneratorExpression ge; - cmsys::auto_ptr cge = - ge.Parse(libDir.c_str()); - this->Internal - ->CachedLinkInterfaceIncludeDirectoriesEntries[config] - .push_back(new cmTargetInternals::TargetPropertyEntry(cge)); + continue; } + + libDir = frameworkCheck.match(1); + + cmGeneratorExpression ge; + cmsys::auto_ptr cge = + ge.Parse(libDir.c_str()); + linkInterfaceIncludeDirectoriesEntries + .push_back(new cmTargetInternals::TargetPropertyEntry(cge)); } } processIncludeDirectories(this, - this->Internal->CachedLinkInterfaceIncludeDirectoriesEntries[config], + linkInterfaceIncludeDirectoriesEntries, includes, uniqueIncludes, &dagChecker, config, debugIncludes); - if (!this->Makefile->IsGeneratingBuildSystem()) - { - deleteAndClear( - this->Internal->CachedLinkInterfaceIncludeDirectoriesEntries); - } - else - { - this->Internal->CacheLinkInterfaceIncludeDirectoriesDone[config] - = true; - } + deleteAndClear(linkInterfaceIncludeDirectoriesEntries); return includes; } @@ -2405,29 +2352,22 @@ void cmTarget::GetCompileOptions(std::vector &result, config, debugOptions); - if (!this->Internal->CacheLinkInterfaceCompileOptionsDone[config]) - { - this->Internal->AddInterfaceEntries( - this, config, "INTERFACE_COMPILE_OPTIONS", - this->Internal->CachedLinkInterfaceCompileOptionsEntries[config]); - } + std::vector + linkInterfaceCompileOptionsEntries; + + this->Internal->AddInterfaceEntries( + this, config, "INTERFACE_COMPILE_OPTIONS", + linkInterfaceCompileOptionsEntries); processCompileOptions(this, - this->Internal->CachedLinkInterfaceCompileOptionsEntries[config], + linkInterfaceCompileOptionsEntries, result, uniqueOptions, &dagChecker, config, debugOptions); - if (!this->Makefile->IsGeneratingBuildSystem()) - { - deleteAndClear(this->Internal->CachedLinkInterfaceCompileOptionsEntries); - } - else - { - this->Internal->CacheLinkInterfaceCompileOptionsDone[config] = true; - } + deleteAndClear(linkInterfaceCompileOptionsEntries); } //---------------------------------------------------------------------------- @@ -2479,66 +2419,54 @@ void cmTarget::GetCompileDefinitions(std::vector &list, config, debugDefines); - if (!this->Internal->CacheLinkInterfaceCompileDefinitionsDone[config]) + std::vector + linkInterfaceCompileDefinitionsEntries; + this->Internal->AddInterfaceEntries( + this, config, "INTERFACE_COMPILE_DEFINITIONS", + linkInterfaceCompileDefinitionsEntries); + if (!config.empty()) { - this->Internal->AddInterfaceEntries( - this, config, "INTERFACE_COMPILE_DEFINITIONS", - this->Internal->CachedLinkInterfaceCompileDefinitionsEntries[config]); - if (!config.empty()) + std::string configPropName = "COMPILE_DEFINITIONS_" + + cmSystemTools::UpperCase(config); + const char *configProp = this->GetProperty(configPropName); + if (configProp) { - std::string configPropName = "COMPILE_DEFINITIONS_" - + cmSystemTools::UpperCase(config); - const char *configProp = this->GetProperty(configPropName); - if (configProp) + switch(this->Makefile->GetPolicyStatus(cmPolicies::CMP0043)) { - switch(this->Makefile->GetPolicyStatus(cmPolicies::CMP0043)) + case cmPolicies::WARN: { - case cmPolicies::WARN: - { - cmOStringStream e; - e << this->Makefile->GetCMakeInstance()->GetPolicies() - ->GetPolicyWarning(cmPolicies::CMP0043); - this->Makefile->IssueMessage(cmake::AUTHOR_WARNING, - e.str()); - } - case cmPolicies::OLD: - { - cmGeneratorExpression ge; - cmsys::auto_ptr cge = - ge.Parse(configProp); - this->Internal - ->CachedLinkInterfaceCompileDefinitionsEntries[config] - .push_back(new cmTargetInternals::TargetPropertyEntry(cge)); - } - break; - case cmPolicies::NEW: - case cmPolicies::REQUIRED_ALWAYS: - case cmPolicies::REQUIRED_IF_USED: - break; + cmOStringStream e; + e << this->Makefile->GetCMakeInstance()->GetPolicies() + ->GetPolicyWarning(cmPolicies::CMP0043); + this->Makefile->IssueMessage(cmake::AUTHOR_WARNING, + e.str()); } + case cmPolicies::OLD: + { + cmGeneratorExpression ge; + cmsys::auto_ptr cge = + ge.Parse(configProp); + linkInterfaceCompileDefinitionsEntries + .push_back(new cmTargetInternals::TargetPropertyEntry(cge)); + } + break; + case cmPolicies::NEW: + case cmPolicies::REQUIRED_ALWAYS: + case cmPolicies::REQUIRED_IF_USED: + break; } } - } processCompileDefinitions(this, - this->Internal->CachedLinkInterfaceCompileDefinitionsEntries[config], + linkInterfaceCompileDefinitionsEntries, list, uniqueOptions, &dagChecker, config, debugDefines); - if (!this->Makefile->IsGeneratingBuildSystem()) - { - deleteAndClear(this->Internal - ->CachedLinkInterfaceCompileDefinitionsEntries); - } - else - { - this->Internal->CacheLinkInterfaceCompileDefinitionsDone[config] - = true; - } + deleteAndClear(linkInterfaceCompileDefinitionsEntries); } //---------------------------------------------------------------------------- @@ -2590,29 +2518,21 @@ void cmTarget::GetCompileFeatures(std::vector &result, config, debugFeatures); - if (!this->Internal->CacheLinkInterfaceCompileFeaturesDone[config]) - { - this->Internal->AddInterfaceEntries( - this, config, "INTERFACE_COMPILE_FEATURES", - this->Internal->CachedLinkInterfaceCompileFeaturesEntries[config]); - } + std::vector + linkInterfaceCompileFeaturesEntries; + this->Internal->AddInterfaceEntries( + this, config, "INTERFACE_COMPILE_FEATURES", + linkInterfaceCompileFeaturesEntries); processCompileFeatures(this, - this->Internal->CachedLinkInterfaceCompileFeaturesEntries[config], + linkInterfaceCompileFeaturesEntries, result, uniqueFeatures, &dagChecker, config, debugFeatures); - if (!this->Makefile->IsGeneratingBuildSystem()) - { - deleteAndClear(this->Internal->CachedLinkInterfaceCompileFeaturesEntries); - } - else - { - this->Internal->CacheLinkInterfaceCompileFeaturesDone[config] = true; - } + deleteAndClear(linkInterfaceCompileFeaturesEntries); } //---------------------------------------------------------------------------- From 133cd996d161cd349e67d90052ac4e20d9488620 Mon Sep 17 00:00:00 2001 From: Brad King Date: Fri, 18 Jul 2014 14:04:56 -0400 Subject: [PATCH 2/2] cmTarget: Drop internal cache of build properties These use a huge amount of memory that accumulates as generation proceeds. On the Unix Makefiles generator, only GetIncludeDirectories and GetCompileDefinitions are even called more than once per target (once for build files, once for dependency scanning preprocessor info). Another approach will be needed to avoid duplicate computation in the cases where it does occur. --- Source/cmTarget.cxx | 134 ++++++++++++++------------------------------ 1 file changed, 43 insertions(+), 91 deletions(-) diff --git a/Source/cmTarget.cxx b/Source/cmTarget.cxx index a812df70f..9d00591e9 100644 --- a/Source/cmTarget.cxx +++ b/Source/cmTarget.cxx @@ -196,11 +196,9 @@ public: public: TargetPropertyEntry(cmsys::auto_ptr cge, cmLinkImplItem const& item = NoLinkImplItem) - : ge(cge), Cached(false), LinkImplItem(item) + : ge(cge), LinkImplItem(item) {} const cmsys::auto_ptr ge; - std::vector CachedEntries; - bool Cached; cmLinkImplItem const& LinkImplItem; }; std::vector IncludeDirectoriesEntries; @@ -635,49 +633,37 @@ static bool processSources(cmTarget const* tgt, for (std::vector::const_iterator it = entries.begin(), end = entries.end(); it != end; ++it) { - bool cacheSources = false; - std::vector entrySources = (*it)->CachedEntries; - if(entrySources.empty()) + std::vector entrySources; + cmSystemTools::ExpandListArgument((*it)->ge->Evaluate(mf, + config, + false, + tgt, + tgt, + dagChecker), + entrySources); + + if ((*it)->ge->GetHadContextSensitiveCondition()) { - cmSystemTools::ExpandListArgument((*it)->ge->Evaluate(mf, - config, - false, - tgt, - tgt, - dagChecker), - entrySources); + contextDependent = true; + } - if ((*it)->ge->GetHadContextSensitiveCondition()) - { - contextDependent = true; - } - else if (mf->IsGeneratingBuildSystem()) - { - cacheSources = true; - } + for(std::vector::iterator i = entrySources.begin(); + i != entrySources.end(); ++i) + { + std::string& src = *i; - for(std::vector::iterator i = entrySources.begin(); - i != entrySources.end(); ++i) + cmSourceFile* sf = mf->GetOrCreateSource(src); + std::string e; + src = sf->GetFullPath(&e); + if(src.empty()) { - std::string& src = *i; - - cmSourceFile* sf = mf->GetOrCreateSource(src); - std::string e; - src = sf->GetFullPath(&e); - if(src.empty()) + if(!e.empty()) { - if(!e.empty()) - { - cmake* cm = mf->GetCMakeInstance(); - cm->IssueMessage(cmake::FATAL_ERROR, e, - tgt->GetBacktrace()); - } - return contextDependent; + cmake* cm = mf->GetCMakeInstance(); + cm->IssueMessage(cmake::FATAL_ERROR, e, + tgt->GetBacktrace()); } - } - if (cacheSources) - { - (*it)->CachedEntries = entrySources; + return contextDependent; } } std::string usedSources; @@ -2003,27 +1989,14 @@ static void processIncludeDirectories(cmTarget const* tgt, std::string const& targetName = item; bool const fromImported = item.Target && item.Target->IsImported(); bool const checkCMP0027 = item.FromGenex; - bool testIsOff = true; - bool cacheIncludes = false; - std::vector& entryIncludes = (*it)->CachedEntries; - if(!entryIncludes.empty()) - { - testIsOff = false; - } - else - { - cmSystemTools::ExpandListArgument((*it)->ge->Evaluate(mf, - config, - false, - tgt, - dagChecker), - entryIncludes); - if (mf->IsGeneratingBuildSystem() - && !(*it)->ge->GetHadContextSensitiveCondition()) - { - cacheIncludes = true; - } - } + std::vector entryIncludes; + cmSystemTools::ExpandListArgument((*it)->ge->Evaluate(mf, + config, + false, + tgt, + dagChecker), + entryIncludes); + std::string usedIncludes; for(std::vector::iterator li = entryIncludes.begin(); li != entryIncludes.end(); ++li) @@ -2105,7 +2078,7 @@ static void processIncludeDirectories(cmTarget const* tgt, } } - if (testIsOff && !cmSystemTools::IsOff(li->c_str())) + if (!cmSystemTools::IsOff(li->c_str())) { cmSystemTools::ConvertToUnixSlashes(*li); } @@ -2120,10 +2093,6 @@ static void processIncludeDirectories(cmTarget const* tgt, } } } - if (cacheIncludes) - { - (*it)->CachedEntries = entryIncludes; - } if (!usedIncludes.empty()) { mf->GetCMakeInstance()->IssueMessage(cmake::LOG, @@ -2229,33 +2198,16 @@ static void processCompileOptionsInternal(cmTarget const* tgt, for (std::vector::const_iterator it = entries.begin(), end = entries.end(); it != end; ++it) { - std::vector& entriesRef = (*it)->CachedEntries; - std::vector localEntries; - std::vector* entryOptions = &entriesRef; - if(!(*it)->Cached) - { - cmSystemTools::ExpandListArgument((*it)->ge->Evaluate(mf, - config, - false, - tgt, - dagChecker), - localEntries); - if (mf->IsGeneratingBuildSystem() - && !(*it)->ge->GetHadContextSensitiveCondition()) - { - // Cache the result. - *entryOptions = localEntries; - (*it)->Cached = true; - } - else - { - // Use the context-sensitive results here. - entryOptions = &localEntries; - } - } + std::vector entryOptions; + cmSystemTools::ExpandListArgument((*it)->ge->Evaluate(mf, + config, + false, + tgt, + dagChecker), + entryOptions); std::string usedOptions; for(std::vector::iterator - li = entryOptions->begin(); li != entryOptions->end(); ++li) + li = entryOptions.begin(); li != entryOptions.end(); ++li) { std::string const& opt = *li;