On Wed, May 21, 2014 at 6:10 PM, Jordan Rose <[email protected]> wrote:
> You might want to bump up the size of the SmallVector too. Right now > you're dynamically allocating a SmallVector<T, 1>, and immediately putting > two things into it. Alternately, you could use a std::vector, which has a > smaller sizeof itself. > I tried the various combinations, and it didn't make a difference, so I went for making the code the simplest, which was reusing the ParentVector typedef. If we change the ParentVector to a SmallVector<T, 2> we'll also use that for getParents() which mostly returns a single element. > > Jordan > > > On May 21, 2014, at 6:28 , Manuel Klimek <[email protected]> wrote: > > > Author: klimek > > Date: Wed May 21 08:28:59 2014 > > New Revision: 209297 > > > > URL: http://llvm.org/viewvc/llvm-project?rev=209297&view=rev > > Log: > > 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. > > > > Modified: > > cfe/trunk/include/clang/AST/ASTContext.h > > cfe/trunk/lib/AST/ASTContext.cpp > > > > Modified: cfe/trunk/include/clang/AST/ASTContext.h > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTContext.h?rev=209297&r1=209296&r2=209297&view=diff > > > ============================================================================== > > --- cfe/trunk/include/clang/AST/ASTContext.h (original) > > +++ cfe/trunk/include/clang/AST/ASTContext.h Wed May 21 08:28:59 2014 > > @@ -424,7 +424,9 @@ public: > > typedef llvm::SmallVector<ast_type_traits::DynTypedNode, 1> > ParentVector; > > > > /// \brief Maps from a node to its parents. > > - typedef llvm::DenseMap<const void *, ParentVector> ParentMap; > > + typedef llvm::DenseMap<const void *, > > + > llvm::PointerUnion<ast_type_traits::DynTypedNode *, > > + ParentVector *>> 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<ParentMap> AllParents; > > > > > > Modified: cfe/trunk/lib/AST/ASTContext.cpp > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTContext.cpp?rev=209297&r1=209296&r2=209297&view=diff > > > ============================================================================== > > --- cfe/trunk/lib/AST/ASTContext.cpp (original) > > +++ cfe/trunk/lib/AST/ASTContext.cpp Wed May 21 08:28:59 2014 > > @@ -755,6 +755,8 @@ ASTContext::ASTContext(LangOptions& LOpt > > } > > > > 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<ast_type_traits::DynTypedNode *>()) { > > + delete Entry.second.get<ast_type_traits::DynTypedNode *>(); > > + } else { > > + assert(Entry.second.is<ParentVector *>()); > > + delete Entry.second.get<ParentVector *>(); > > + } > > + } > > +} > > + > > 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<ast_type_traits::DynTypedNode *>()) > { > > + auto *Node = > > + NodeOrVector.template get<ast_type_traits::DynTypedNode > *>(); > > + auto *Vector = new ASTContext::ParentVector(1, *Node); > > + Vector->push_back(ParentStack.back()); > > + NodeOrVector = Vector; > > + delete Node; > > + } else { > > + assert(NodeOrVector.template is<ASTContext::ParentVector > *>()); > > + NodeOrVector.template get<ASTContext::ParentVector > *>()->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_tr > > if (I == AllParents->end()) { > > return ParentVector(); > > } > > - return I->second; > > + if (I->second.is<ast_type_traits::DynTypedNode *>()) { > > + return ParentVector(1, *I->second.get<ast_type_traits::DynTypedNode > *>()); > > + } > > + const auto &Parents = *I->second.get<ParentVector *>(); > > + return ParentVector(Parents.begin(), Parents.end()); > > } > > > > bool > > > > > > _______________________________________________ > > cfe-commits mailing list > > [email protected] > > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits > >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
