From 95403e6f607cb51a4c23663c80712db2af2a1cff Mon Sep 17 00:00:00 2001 From: Manuel Klimek Date: Wed, 21 May 2014 13:28:59 +0000 Subject: [PATCH] Make the parent-map use significantly less memory. On test files I ran this on, memory consumption overall went down from 2.5G to 2G, without performance regressions. I also investigated making DynTypedNode by itself smaller (by pulling out pointers for everything that doesn't fit in 8 bytes). This led to another 200-300MB saved, but also introduced a significant regression in performance due to the memory management overhead. llvm-svn: 209297 --- clang/include/clang/AST/ASTContext.h | 5 +++- clang/lib/AST/ASTContext.cpp | 40 +++++++++++++++++++++++++--- 2 files changed, 41 insertions(+), 4 deletions(-) diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h index 229ac9fba976..1f97a4c6f0b1 100644 --- a/clang/include/clang/AST/ASTContext.h +++ b/clang/include/clang/AST/ASTContext.h @@ -424,7 +424,9 @@ public: typedef llvm::SmallVector ParentVector; /// \brief Maps from a node to its parents. - typedef llvm::DenseMap ParentMap; + typedef llvm::DenseMap> ParentMap; /// \brief Returns the parents of the given node. /// @@ -2293,6 +2295,7 @@ private: friend class DeclContext; friend class DeclarationNameTable; void ReleaseDeclContextMaps(); + void ReleaseParentMapEntries(); std::unique_ptr AllParents; diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp index 99547f817582..2ea93bae674b 100644 --- a/clang/lib/AST/ASTContext.cpp +++ b/clang/lib/AST/ASTContext.cpp @@ -755,6 +755,8 @@ ASTContext::ASTContext(LangOptions& LOpts, SourceManager &SM, } ASTContext::~ASTContext() { + ReleaseParentMapEntries(); + // Release the DenseMaps associated with DeclContext objects. // FIXME: Is this the ideal solution? ReleaseDeclContextMaps(); @@ -789,6 +791,18 @@ ASTContext::~ASTContext() { llvm::DeleteContainerSeconds(MangleNumberingContexts); } +void ASTContext::ReleaseParentMapEntries() { + if (!AllParents) return; + for (const auto &Entry : *AllParents) { + if (Entry.second.is()) { + delete Entry.second.get(); + } else { + assert(Entry.second.is()); + delete Entry.second.get(); + } + } +} + void ASTContext::AddDeallocation(void (*Callback)(void*), void *Data) { Deallocations[Callback].push_back(Data); } @@ -8162,7 +8176,7 @@ namespace { bool TraverseNode(T *Node, bool(VisitorBase:: *traverse) (T *)) { if (!Node) return true; - if (ParentStack.size() > 0) + if (ParentStack.size() > 0) { // FIXME: Currently we add the same parent multiple times, for example // when we visit all subexpressions of template instantiations; this is // suboptimal, bug benign: the only way to visit those is with @@ -8171,7 +8185,23 @@ namespace { // map. The main problem there is to implement hash functions / // comparison operators for all types that DynTypedNode supports that // do not have pointer identity. - (*Parents)[Node].push_back(ParentStack.back()); + auto &NodeOrVector = (*Parents)[Node]; + if (NodeOrVector.isNull()) { + NodeOrVector = new ast_type_traits::DynTypedNode(ParentStack.back()); + } else if (NodeOrVector + .template is()) { + auto *Node = + NodeOrVector.template get(); + auto *Vector = new ASTContext::ParentVector(1, *Node); + Vector->push_back(ParentStack.back()); + NodeOrVector = Vector; + delete Node; + } else { + assert(NodeOrVector.template is()); + NodeOrVector.template get()->push_back( + ParentStack.back()); + } + } ParentStack.push_back(ast_type_traits::DynTypedNode::create(*Node)); bool Result = (this ->* traverse) (Node); ParentStack.pop_back(); @@ -8209,7 +8239,11 @@ ASTContext::getParents(const ast_type_traits::DynTypedNode &Node) { if (I == AllParents->end()) { return ParentVector(); } - return I->second; + if (I->second.is()) { + return ParentVector(1, *I->second.get()); + } + const auto &Parents = *I->second.get(); + return ParentVector(Parents.begin(), Parents.end()); } bool