r209297 - Make the parent-map use significantly less memory.

[Jordan Rose]

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.

Jordan


On May 21, 2014, at 6:28 , Manuel Klimek <klimek at google.com> 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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits