Author: Daniel Grumberg Date: 2022-03-24T17:44:00Z New Revision: 0ee06c31aa57576c05c7ca3e68d2cac9ddf59811
URL: https://github.com/llvm/llvm-project/commit/0ee06c31aa57576c05c7ca3e68d2cac9ddf59811 DIFF: https://github.com/llvm/llvm-project/commit/0ee06c31aa57576c05c7ca3e68d2cac9ddf59811.diff LOG: [clang][extract-api] Stop allocating APIRecords via BumpPtrAllocator Using a BumpPtrAllocator introduced memory leaks for APIRecords as they contain a std::vector. This meant that we needed to always keep a reference to the records in APISet and arrange for their destructor to get called appropriately. This was further complicated by the need for records to own sub-records as these subrecords would still need to be allocated via the BumpPtrAllocator and the owning record would now need to arrange for the destructor of its subrecords to be called appropriately. Since APIRecords contain a std::vector so whenever elements get added to that there is an associated heap allocation regardless. Since performance isn't currently our main priority it makes sense to use regular unique_ptr to keep track of APIRecords, this way we don't need to arrange for destructors to get called. The BumpPtrAllocator is still used for strings such as USRs so that we can easily de-duplicate them as necessary. Differential Revision: https://reviews.llvm.org/D122331 Added: Modified: clang/include/clang/ExtractAPI/API.h clang/lib/ExtractAPI/API.cpp Removed: ################################################################################ diff --git a/clang/include/clang/ExtractAPI/API.h b/clang/include/clang/ExtractAPI/API.h index e8f9219358ccf..beedda6464438 100644 --- a/clang/include/clang/ExtractAPI/API.h +++ b/clang/include/clang/ExtractAPI/API.h @@ -30,25 +30,6 @@ #include "llvm/Support/Casting.h" #include <memory> -namespace { - -/// \brief A custom deleter used for ``std::unique_ptr`` to APIRecords stored -/// in the BumpPtrAllocator. -/// -/// \tparam T the exact type of the APIRecord subclass. -template <typename T> struct UniquePtrBumpPtrAllocatorDeleter { - void operator()(T *Instance) { Instance->~T(); } -}; - -/// A unique pointer to an APIRecord stored in the BumpPtrAllocator. -/// -/// \tparam T the exact type of the APIRecord subclass. -template <typename T> -using APIRecordUniquePtr = - std::unique_ptr<T, UniquePtrBumpPtrAllocatorDeleter<T>>; - -} // anonymous namespace - namespace clang { namespace extractapi { @@ -165,7 +146,7 @@ struct EnumConstantRecord : APIRecord { /// This holds information associated with enums. struct EnumRecord : APIRecord { - SmallVector<APIRecordUniquePtr<EnumConstantRecord>> Constants; + SmallVector<std::unique_ptr<EnumConstantRecord>> Constants; EnumRecord(StringRef Name, StringRef USR, PresumedLoc Loc, const AvailabilityInfo &Availability, const DocComment &Comment, @@ -194,7 +175,7 @@ struct StructFieldRecord : APIRecord { /// This holds information associated with structs. struct StructRecord : APIRecord { - SmallVector<APIRecordUniquePtr<StructFieldRecord>> Fields; + SmallVector<std::unique_ptr<StructFieldRecord>> Fields; StructRecord(StringRef Name, StringRef USR, PresumedLoc Loc, const AvailabilityInfo &Availability, const DocComment &Comment, @@ -302,17 +283,16 @@ class APISet { /// A map to store the set of GlobalRecord%s with the declaration name as the /// key. using GlobalRecordMap = - llvm::MapVector<StringRef, APIRecordUniquePtr<GlobalRecord>>; + llvm::MapVector<StringRef, std::unique_ptr<GlobalRecord>>; /// A map to store the set of EnumRecord%s with the declaration name as the /// key. - using EnumRecordMap = - llvm::MapVector<StringRef, APIRecordUniquePtr<EnumRecord>>; + using EnumRecordMap = llvm::MapVector<StringRef, std::unique_ptr<EnumRecord>>; /// A map to store the set of StructRecord%s with the declaration name as the /// key. using StructRecordMap = - llvm::MapVector<StringRef, APIRecordUniquePtr<StructRecord>>; + llvm::MapVector<StringRef, std::unique_ptr<StructRecord>>; /// Get the target triple for the ExtractAPI invocation. const llvm::Triple &getTarget() const { return Target; } @@ -340,8 +320,10 @@ class APISet { : Target(Target), LangOpts(LangOpts) {} private: - /// BumpPtrAllocator to store APIRecord%s and generated/copied strings. - llvm::BumpPtrAllocator Allocator; + /// BumpPtrAllocator to store generated/copied strings. + /// + /// Note: The main use for this is being able to deduplicate strings. + llvm::BumpPtrAllocator StringAllocator; const llvm::Triple Target; const LangOptions LangOpts; diff --git a/clang/lib/ExtractAPI/API.cpp b/clang/lib/ExtractAPI/API.cpp index 7aa82336ee2ca..6b60d2c2a1b42 100644 --- a/clang/lib/ExtractAPI/API.cpp +++ b/clang/lib/ExtractAPI/API.cpp @@ -17,7 +17,7 @@ #include "clang/AST/CommentLexer.h" #include "clang/AST/RawCommentList.h" #include "clang/Index/USRGeneration.h" -#include "llvm/Support/Allocator.h" +#include <memory> using namespace clang::extractapi; using namespace llvm; @@ -32,9 +32,9 @@ GlobalRecord *APISet::addGlobal(GVKind Kind, StringRef Name, StringRef USR, auto Result = Globals.insert({Name, nullptr}); if (Result.second) { // Create the record if it does not already exist. - auto Record = APIRecordUniquePtr<GlobalRecord>(new (Allocator) GlobalRecord{ + auto Record = std::make_unique<GlobalRecord>( Kind, Name, USR, Loc, Availability, Linkage, Comment, Fragments, - SubHeading, Signature}); + SubHeading, Signature); Result.first->second = std::move(Record); } return Result.first->second.get(); @@ -63,9 +63,8 @@ EnumConstantRecord *APISet::addEnumConstant( EnumRecord *Enum, StringRef Name, StringRef USR, PresumedLoc Loc, const AvailabilityInfo &Availability, const DocComment &Comment, DeclarationFragments Declaration, DeclarationFragments SubHeading) { - auto Record = - APIRecordUniquePtr<EnumConstantRecord>(new (Allocator) EnumConstantRecord{ - Name, USR, Loc, Availability, Comment, Declaration, SubHeading}); + auto Record = std::make_unique<EnumConstantRecord>( + Name, USR, Loc, Availability, Comment, Declaration, SubHeading); return Enum->Constants.emplace_back(std::move(Record)).get(); } @@ -77,8 +76,8 @@ EnumRecord *APISet::addEnum(StringRef Name, StringRef USR, PresumedLoc Loc, auto Result = Enums.insert({Name, nullptr}); if (Result.second) { // Create the record if it does not already exist. - auto Record = APIRecordUniquePtr<EnumRecord>(new (Allocator) EnumRecord{ - Name, USR, Loc, Availability, Comment, Declaration, SubHeading}); + auto Record = std::make_unique<EnumRecord>( + Name, USR, Loc, Availability, Comment, Declaration, SubHeading); Result.first->second = std::move(Record); } return Result.first->second.get(); @@ -90,9 +89,8 @@ StructFieldRecord *APISet::addStructField(StructRecord *Struct, StringRef Name, const DocComment &Comment, DeclarationFragments Declaration, DeclarationFragments SubHeading) { - auto Record = - APIRecordUniquePtr<StructFieldRecord>(new (Allocator) StructFieldRecord{ - Name, USR, Loc, Availability, Comment, Declaration, SubHeading}); + auto Record = std::make_unique<StructFieldRecord>( + Name, USR, Loc, Availability, Comment, Declaration, SubHeading); return Struct->Fields.emplace_back(std::move(Record)).get(); } @@ -104,8 +102,8 @@ StructRecord *APISet::addStruct(StringRef Name, StringRef USR, PresumedLoc Loc, auto Result = Structs.insert({Name, nullptr}); if (Result.second) { // Create the record if it does not already exist. - auto Record = APIRecordUniquePtr<StructRecord>(new (Allocator) StructRecord{ - Name, USR, Loc, Availability, Comment, Declaration, SubHeading}); + auto Record = std::make_unique<StructRecord>( + Name, USR, Loc, Availability, Comment, Declaration, SubHeading); Result.first->second = std::move(Record); } return Result.first->second.get(); @@ -122,10 +120,10 @@ StringRef APISet::copyString(StringRef String) { return {}; // No need to allocate memory and copy if the string has already been stored. - if (Allocator.identifyObject(String.data())) + if (StringAllocator.identifyObject(String.data())) return String; - void *Ptr = Allocator.Allocate(String.size(), 1); + void *Ptr = StringAllocator.Allocate(String.size(), 1); memcpy(Ptr, String.data(), String.size()); return StringRef(reinterpret_cast<const char *>(Ptr), String.size()); } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
