From ff5a01a7b1f2d14035e638f5b67e982a9b9e50ac Mon Sep 17 00:00:00 2001
From: Chandler Carruth <chandlerc@gmail.com>
Date: Wed, 30 Dec 2015 03:00:23 +0000
Subject: [PATCH] [ptr-traits] Switch from a really wasteful SmallDenseMap of
SmallVector<.., 16> (16!!!!) objects to a simple SmallVector of pairs.
This no longer de-duplicates the common function pointers used during
deallocation, but this doesn't really seem worth the complexity and
overhead of managing the map-of-vectors. Notably, there is no reason to
assume that functions have the 4-byte alignment that DenseMap relies on,
and indeed this prevents checking the alignment of the DenseMap keys
because we can't even meaningfully query the alignment of functions
using our existing alignment tools.
Generally, function pointers don't seem like a great idea for keys in
a DenseMap. =]
I chatted with Richard Smith about this a bit as well and have written
down a FIXME because this *does* waste some memory and in general seems
a very clumsy memory management strategy. He would like to see a more
fundamental fix eventually here that tries to build a better pattern.
llvm-svn: 256610
---
clang/include/clang/AST/ASTContext.h | 12 +++++++++---
clang/lib/AST/ASTContext.cpp | 8 +++-----
2 files changed, 12 insertions(+), 8 deletions(-)
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index b24e7652312e..44b5e3989b2d 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -2514,9 +2514,15 @@ private:
/// \brief A set of deallocations that should be performed when the
/// ASTContext is destroyed.
- typedef llvm::SmallDenseMap<void(*)(void*), llvm::SmallVector<void*, 16> >
- DeallocationMap;
- DeallocationMap Deallocations;
+ // FIXME: We really should have a better mechanism in the ASTContext to
+ // manage running destructors for types which do variable sized allocation
+ // within the AST. In some places we thread the AST bump pointer allocator
+ // into the datastructures which avoids this mess during deallocation but is
+ // wasteful of memory, and here we require a lot of error prone book keeping
+ // in order to track and run destructors while we're tearing things down.
+ typedef llvm::SmallVector<std::pair<void (*)(void *), void *>, 16>
+ DeallocationFunctionsAndArguments;
+ DeallocationFunctionsAndArguments Deallocations;
// FIXME: This currently contains the set of StoredDeclMaps used
// by DeclContext objects. This probably should not be in ASTContext,
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index b344b0687f2a..508124a2a6eb 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -760,10 +760,8 @@ ASTContext::~ASTContext() {
ReleaseDeclContextMaps();
// Call all of the deallocation functions on all of their targets.
- for (DeallocationMap::const_iterator I = Deallocations.begin(),
- E = Deallocations.end(); I != E; ++I)
- for (unsigned J = 0, N = I->second.size(); J != N; ++J)
- (I->first)((I->second)[J]);
+ for (auto &Pair : Deallocations)
+ (Pair.first)(Pair.second);
// ASTRecordLayout objects in ASTRecordLayouts must always be destroyed
// because they can contain DenseMaps.
@@ -812,7 +810,7 @@ void ASTContext::ReleaseParentMapEntries() {
}
void ASTContext::AddDeallocation(void (*Callback)(void*), void *Data) {
- Deallocations[Callback].push_back(Data);
+ Deallocations.push_back({Callback, Data});
}
void