Author: Paul Kirth Date: 2026-04-03T20:33:56Z New Revision: 1deab59f6f82e75c6f3afd895a222e328ec51150
URL: https://github.com/llvm/llvm-project/commit/1deab59f6f82e75c6f3afd895a222e328ec51150 DIFF: https://github.com/llvm/llvm-project/commit/1deab59f6f82e75c6f3afd895a222e328ec51150.diff LOG: [clang-doc] Migrate Namespaces to arena allocation (#190048) This patch allocates the NamespaceInfo types in the local arenas, and adapts the merging logic for the new list type and its children. Memory use and performance improve slightly. Micro-benchmarks show a regression in merge operations due to the more complex list operations. ## Build Clang-Doc Documentation | Metric | Baseline | Prev | This | Culm% | Seq% | | :--- | :--- | :--- | :--- | :--- | :--- | | Time | 920.5s | 1009.2s | 1002.4s | +8.9% | -0.7% | | Memory | 86.0G | 43.2G | 43.9G | -49.0% | +1.6% | ## Microbenchmarks (Filtered for >1% Delta) | Benchmark | Baseline | Prev | This | Culm% | Seq% | | :--- | :--- | :--- | :--- | :--- | :--- | | BM_BitcodeReader_Scale/10 | 67.9us | 69.7us | 69.3us | +1.9% | -0.7% | | BM_BitcodeReader_Scale/10000 | 70.5ms | 22.3ms | 24.8ms | -64.8% | +11.4% | | BM_BitcodeReader_Scale/4096 | 23.2ms | 4.7ms | 4.4ms | -80.9% | -5.7% | | BM_BitcodeReader_Scale/512 | 509.4us | 558.7us | 540.2us | +6.0% | -3.3% | | BM_BitcodeReader_Scale/64 | 114.8us | 119.2us | 117.0us | +1.9% | -1.8% | | BM_EmitInfoFunction | 1.6us | 1.6us | 1.6us | -1.1% | +0.8% | | BM_Index_Insertion/10 | 2.3us | 4.2us | 3.7us | +61.7% | -11.3% | | BM_Index_Insertion/10000 | 3.1ms | 5.4ms | 4.9ms | +56.8% | -9.1% | | BM_Index_Insertion/4096 | 1.3ms | 2.2ms | 2.0ms | +54.5% | -8.2% | | BM_Index_Insertion/512 | 153.6us | 259.6us | 236.7us | +54.1% | -8.8% | | BM_Index_Insertion/64 | 18.1us | 30.7us | 27.9us | +54.5% | -9.0% | | BM_JSONGenerator_Scale/10 | 36.8us | 37.6us | 36.6us | -0.7% | -2.7% | | BM_JSONGenerator_Scale/4096 | 33.7ms | 34.3ms | 34.2ms | +1.4% | -0.3% | | BM_JSONGenerator_Scale/64 | 222.4us | 225.6us | 220.2us | -1.0% | -2.4% | | BM_Mapper_Scale/10 | 2.5ms | 2.5ms | 2.5ms | -1.4% | -1.7% | | BM_Mapper_Scale/10000 | 104.3ms | 109.3ms | 105.9ms | +1.6% | -3.1% | | BM_Mapper_Scale/4096 | 44.3ms | 43.9ms | 44.7ms | +0.8% | +1.8% | | BM_Mapper_Scale/512 | 7.6ms | 7.6ms | 7.6ms | +0.8% | +1.2% | | BM_MergeInfos_Scale/10000 | 12.2ms | 1.6ms | 2.0ms | -83.6% | +28.7% | | BM_MergeInfos_Scale/2 | 1.9us | 1.7us | 1.7us | -8.1% | +0.9% | | BM_MergeInfos_Scale/4096 | 2.8ms | 520.5us | 557.4us | -79.9% | +7.1% | | BM_MergeInfos_Scale/512 | 68.9us | 37.9us | 39.9us | -42.0% | +5.3% | | BM_MergeInfos_Scale/64 | 10.3us | 6.3us | 6.5us | -36.7% | +2.7% | | BM_MergeInfos_Scale/8 | 2.8us | 2.2us | 2.2us | -20.7% | +1.2% | | BM_SerializeFunctionInfo | 25.5us | 25.8us | 26.1us | +2.1% | +1.1% | Added: Modified: clang-tools-extra/clang-doc/BitcodeReader.cpp clang-tools-extra/clang-doc/JSONGenerator.cpp clang-tools-extra/clang-doc/Representation.cpp clang-tools-extra/clang-doc/Representation.h clang-tools-extra/clang-doc/Serialize.cpp clang-tools-extra/clang-doc/YAMLGenerator.cpp clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp clang-tools-extra/unittests/clang-doc/JSONGeneratorTest.cpp clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp clang-tools-extra/unittests/clang-doc/MergeTest.cpp clang-tools-extra/unittests/clang-doc/SerializeTest.cpp clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp Removed: ################################################################################ diff --git a/clang-tools-extra/clang-doc/BitcodeReader.cpp b/clang-tools-extra/clang-doc/BitcodeReader.cpp index 74f67032e068f..ddb1bccdd5469 100644 --- a/clang-tools-extra/clang-doc/BitcodeReader.cpp +++ b/clang-tools-extra/clang-doc/BitcodeReader.cpp @@ -720,9 +720,11 @@ llvm::Error addReference(NamespaceInfo *I, Reference &&R, FieldId F) { case FieldId::F_namespace: I->Namespace.emplace_back(std::move(R)); return llvm::Error::success(); - case FieldId::F_child_namespace: - I->Children.Namespaces.emplace_back(std::move(R)); + case FieldId::F_child_namespace: { + Reference *NewR = allocatePtr<Reference>(TransientArena, std::move(R)); + I->Children.Namespaces.push_back(*NewR); return llvm::Error::success(); + } case FieldId::F_child_record: I->Children.Records.emplace_back(std::move(R)); return llvm::Error::success(); diff --git a/clang-tools-extra/clang-doc/JSONGenerator.cpp b/clang-tools-extra/clang-doc/JSONGenerator.cpp index c753da33299af..8db7eaafcfb30 100644 --- a/clang-tools-extra/clang-doc/JSONGenerator.cpp +++ b/clang-tools-extra/clang-doc/JSONGenerator.cpp @@ -493,13 +493,16 @@ static void serializeArray(const Container &Records, Object &Obj, StringRef Key, json::Value RecordsArray = Array(); auto &RecordsArrayRef = *RecordsArray.getAsArray(); RecordsArrayRef.reserve(Records.size()); - for (size_t Index = 0; Index < Records.size(); ++Index) { + size_t Index = 0; + size_t Size = Records.size(); + for (const auto &Item : Records) { json::Value ItemVal = Object(); auto &ItemObj = *ItemVal.getAsObject(); - SerializeInfo(Records[Index], ItemObj); - if (Index == Records.size() - 1) + SerializeInfo(Item, ItemObj); + if (Index == Size - 1) ItemObj[EndKey] = true; RecordsArrayRef.push_back(ItemVal); + ++Index; } Obj[Key] = RecordsArray; UpdateJson(Obj); diff --git a/clang-tools-extra/clang-doc/Representation.cpp b/clang-tools-extra/clang-doc/Representation.cpp index 0467e375d8cc9..1535fcc71df66 100644 --- a/clang-tools-extra/clang-doc/Representation.cpp +++ b/clang-tools-extra/clang-doc/Representation.cpp @@ -116,6 +116,23 @@ static int getChildIndexIfExists(OwningVec<T> &Children, T &ChildToMerge) { return -1; } +template <typename T> +static void reduceChildren(llvm::simple_ilist<T> &Children, + llvm::simple_ilist<T> &&ChildrenToMerge) { + while (!ChildrenToMerge.empty()) { + T *ChildToMerge = &ChildrenToMerge.front(); + ChildrenToMerge.pop_front(); + + auto It = llvm::find_if( + Children, [&](const T &C) { return C.USR == ChildToMerge->USR; }); + if (It == Children.end()) { + Children.push_back(*ChildToMerge); + } else { + It->merge(std::move(*ChildToMerge)); + } + } +} + template <typename T> static void reduceChildren(OwningVec<T> &Children, OwningVec<T> &&ChildrenToMerge) { @@ -510,7 +527,7 @@ ClangDocContext::ClangDocContext(tooling::ExecutionContext *ECtx, } void ScopeChildren::sort() { - llvm::sort(Namespaces); + Namespaces.sort(); llvm::sort(Records); llvm::sort(Functions); llvm::sort(Enums); diff --git a/clang-tools-extra/clang-doc/Representation.h b/clang-tools-extra/clang-doc/Representation.h index c8f3fc946f064..f54d94ba84aaf 100644 --- a/clang-tools-extra/clang-doc/Representation.h +++ b/clang-tools-extra/clang-doc/Representation.h @@ -21,6 +21,7 @@ #include "llvm/ADT/SmallString.h" #include "llvm/ADT/SmallVector.h" #include "llvm/ADT/ilist_node.h" +#include "llvm/ADT/simple_ilist.h" #include "llvm/Support/Allocator.h" #include "llvm/Support/Mutex.h" #include "llvm/Support/StringSaver.h" @@ -107,6 +108,11 @@ OwnedPtr<T> allocatePtr(Args &&...args) { return std::make_unique<T>(std::forward<Args>(args)...); } +template <typename T, typename... Args> +T *allocatePtr(llvm::BumpPtrAllocator &Alloc, Args &&...args) { + return new (Alloc.Allocate<T>()) T(std::forward<Args>(args)...); +} + // A helper function to access the underlying pointer from an owned pointer, // abstracting away the pointer dereferencing mechanism. template <typename T> T *getPtr(const OwnedPtr<T> &O) { return O.get(); } @@ -197,7 +203,7 @@ struct CommentInfo : public llvm::ilist_node<CommentInfo> { // (for (T)ParamCommand). }; -struct Reference { +struct Reference : public llvm::ilist_node<Reference> { // This variant (that takes no qualified name parameter) uses the Name as the // QualName (very useful in unit tests to reduce verbosity). This can't use an // empty string to indicate the default because we need to accept the empty @@ -273,7 +279,7 @@ struct ScopeChildren { // // Namespaces are not syntactically valid as children of records, but making // this general for all possible container types reduces code complexity. - OwningVec<Reference> Namespaces; + llvm::simple_ilist<Reference> Namespaces; OwningVec<Reference> Records; OwningVec<FunctionInfo> Functions; OwningVec<EnumInfo> Enums; diff --git a/clang-tools-extra/clang-doc/Serialize.cpp b/clang-tools-extra/clang-doc/Serialize.cpp index cb5c5ef03e197..328b8c8a0c9ab 100644 --- a/clang-tools-extra/clang-doc/Serialize.cpp +++ b/clang-tools-extra/clang-doc/Serialize.cpp @@ -458,8 +458,10 @@ bool Serializer::shouldSerializeInfo(bool PublicOnly, // // See MakeAndInsertIntoParent(). void Serializer::InsertChild(ScopeChildren &Scope, const NamespaceInfo &Info) { - Scope.Namespaces.emplace_back(Info.USR, Info.Name, InfoType::IT_namespace, - Info.Name, getInfoRelativePath(Info.Namespace)); + Reference *R = allocatePtr<Reference>(TransientArena, Info.USR, Info.Name, + InfoType::IT_namespace, Info.Name, + getInfoRelativePath(Info.Namespace)); + Scope.Namespaces.push_back(*R); } void Serializer::InsertChild(ScopeChildren &Scope, const RecordInfo &Info) { diff --git a/clang-tools-extra/clang-doc/YAMLGenerator.cpp b/clang-tools-extra/clang-doc/YAMLGenerator.cpp index 390d225bded03..fc814c09c7f3a 100644 --- a/clang-tools-extra/clang-doc/YAMLGenerator.cpp +++ b/clang-tools-extra/clang-doc/YAMLGenerator.cpp @@ -328,8 +328,10 @@ template <> struct MappingTraits<MemberTypeInfo> { template <> struct MappingTraits<NamespaceInfo> { static void mapping(IO &IO, NamespaceInfo &I) { infoMapping(IO, I); - IO.mapOptional("ChildNamespaces", I.Children.Namespaces, - OwningVec<Reference>()); + std::vector<Reference> TempNamespaces; + for (const auto &N : I.Children.Namespaces) + TempNamespaces.push_back(N); + IO.mapOptional("ChildNamespaces", TempNamespaces, std::vector<Reference>()); IO.mapOptional("ChildRecords", I.Children.Records, OwningVec<Reference>()); IO.mapOptional("ChildFunctions", I.Children.Functions); IO.mapOptional("ChildEnums", I.Children.Enums); diff --git a/clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp b/clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp index fa46f79081e80..7b117a96c53fe 100644 --- a/clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp +++ b/clang-tools-extra/unittests/clang-doc/BitcodeTest.cpp @@ -70,8 +70,8 @@ TEST_F(BitcodeTest, emitNamespaceInfoBitcode) { I.Name = "r"; I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace); - I.Children.Namespaces.emplace_back(EmptySID, "ChildNamespace", - InfoType::IT_namespace); + Reference NewNamespace(EmptySID, "ChildNamespace", InfoType::IT_namespace); + I.Children.Namespaces.push_back(NewNamespace); I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record); I.Children.Functions.emplace_back(); I.Children.Enums.emplace_back(); diff --git a/clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp b/clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp index a86ebda5c6f5d..92127da53b7bb 100644 --- a/clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp +++ b/clang-tools-extra/unittests/clang-doc/ClangDocTest.cpp @@ -177,9 +177,13 @@ void CheckNamespaceInfo(NamespaceInfo *Expected, NamespaceInfo *Actual) { ASSERT_EQ(Expected->Children.Namespaces.size(), Actual->Children.Namespaces.size()); - for (size_t Idx = 0; Idx < Actual->Children.Namespaces.size(); ++Idx) - CheckReference(Expected->Children.Namespaces[Idx], - Actual->Children.Namespaces[Idx]); + auto ItExpected = Expected->Children.Namespaces.begin(); + auto ItActual = Actual->Children.Namespaces.begin(); + while (ItExpected != Expected->Children.Namespaces.end()) { + CheckReference(*ItExpected, *ItActual); + ++ItExpected; + ++ItActual; + } ASSERT_EQ(Expected->Children.Records.size(), Actual->Children.Records.size()); for (size_t Idx = 0; Idx < Actual->Children.Records.size(); ++Idx) diff --git a/clang-tools-extra/unittests/clang-doc/JSONGeneratorTest.cpp b/clang-tools-extra/unittests/clang-doc/JSONGeneratorTest.cpp index ae2a0e0f23f00..6edd303dbc326 100644 --- a/clang-tools-extra/unittests/clang-doc/JSONGeneratorTest.cpp +++ b/clang-tools-extra/unittests/clang-doc/JSONGeneratorTest.cpp @@ -199,9 +199,10 @@ TEST_F(JSONGeneratorTest, emitNamespaceJSON) { I.Path = "path/to/A"; I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace); - I.Children.Namespaces.emplace_back( - EmptySID, "ChildNamespace", InfoType::IT_namespace, - "path::to::A::Namespace::ChildNamespace", "path/to/A/Namespace"); + Reference NewNamespace(EmptySID, "ChildNamespace", InfoType::IT_namespace, + "path::to::A::Namespace::ChildNamespace", + "path/to/A/Namespace"); + I.Children.Namespaces.push_back(NewNamespace); I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record, "path::to::A::Namespace::ChildStruct", "path/to/A/Namespace"); diff --git a/clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp b/clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp index 0f6c0eaa00c21..12eec3f491fd0 100644 --- a/clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp +++ b/clang-tools-extra/unittests/clang-doc/MDGeneratorTest.cpp @@ -28,8 +28,8 @@ TEST_F(MDGeneratorTest, emitNamespaceMD) { I.Name = "Namespace"; I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace); - I.Children.Namespaces.emplace_back(EmptySID, "ChildNamespace", - InfoType::IT_namespace); + Reference NewNamespace(EmptySID, "ChildNamespace", InfoType::IT_namespace); + I.Children.Namespaces.push_back(NewNamespace); I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record); I.Children.Functions.emplace_back(); I.Children.Functions.back().Name = "OneFunction"; diff --git a/clang-tools-extra/unittests/clang-doc/MergeTest.cpp b/clang-tools-extra/unittests/clang-doc/MergeTest.cpp index 344f1bfd74f94..b42c95c861b61 100644 --- a/clang-tools-extra/unittests/clang-doc/MergeTest.cpp +++ b/clang-tools-extra/unittests/clang-doc/MergeTest.cpp @@ -20,8 +20,8 @@ TEST_F(MergeTest, mergeNamespaceInfos) { One.Name = "Namespace"; One.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace); - One.Children.Namespaces.emplace_back(NonEmptySID, "ChildNamespace", - InfoType::IT_namespace); + Reference RA(NonEmptySID, "ChildNamespace", InfoType::IT_namespace); + One.Children.Namespaces.push_back(RA); One.Children.Records.emplace_back(NonEmptySID, "ChildStruct", InfoType::IT_record); One.Children.Functions.emplace_back(); @@ -35,8 +35,8 @@ TEST_F(MergeTest, mergeNamespaceInfos) { Two.Name = "Namespace"; Two.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace); - Two.Children.Namespaces.emplace_back(EmptySID, "OtherChildNamespace", - InfoType::IT_namespace); + Reference RB(EmptySID, "OtherChildNamespace", InfoType::IT_namespace); + Two.Children.Namespaces.push_back(RB); Two.Children.Records.emplace_back(EmptySID, "OtherChildStruct", InfoType::IT_record); Two.Children.Functions.emplace_back(); @@ -52,12 +52,12 @@ TEST_F(MergeTest, mergeNamespaceInfos) { Expected->Name = "Namespace"; Expected->Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace); - Expected->Children.Namespaces.emplace_back(NonEmptySID, "ChildNamespace", - InfoType::IT_namespace); + Reference RC(NonEmptySID, "ChildNamespace", InfoType::IT_namespace); + Expected->Children.Namespaces.push_back(RC); Expected->Children.Records.emplace_back(NonEmptySID, "ChildStruct", InfoType::IT_record); - Expected->Children.Namespaces.emplace_back(EmptySID, "OtherChildNamespace", - InfoType::IT_namespace); + Reference RD(EmptySID, "OtherChildNamespace", InfoType::IT_namespace); + Expected->Children.Namespaces.push_back(RD); Expected->Children.Records.emplace_back(EmptySID, "OtherChildStruct", InfoType::IT_record); Expected->Children.Functions.emplace_back(); diff --git a/clang-tools-extra/unittests/clang-doc/SerializeTest.cpp b/clang-tools-extra/unittests/clang-doc/SerializeTest.cpp index 5612e55343382..4e963fd64f11b 100644 --- a/clang-tools-extra/unittests/clang-doc/SerializeTest.cpp +++ b/clang-tools-extra/unittests/clang-doc/SerializeTest.cpp @@ -600,14 +600,14 @@ TEST_F(SerializeTest, emitChildNamespaces) { NamespaceInfo *ParentA = InfoAsNamespace(Infos[1].get()); NamespaceInfo ExpectedParentA(EmptySID); - ExpectedParentA.Children.Namespaces.emplace_back(EmptySID, "A", - InfoType::IT_namespace); + Reference RA(EmptySID, "A", InfoType::IT_namespace); + ExpectedParentA.Children.Namespaces.push_back(RA); CheckNamespaceInfo(&ExpectedParentA, ParentA); NamespaceInfo *ParentB = InfoAsNamespace(Infos[3].get()); NamespaceInfo ExpectedParentB(EmptySID); - ExpectedParentB.Children.Namespaces.emplace_back( - EmptySID, "B", InfoType::IT_namespace, "A::B", "A"); + Reference RB(EmptySID, "B", InfoType::IT_namespace, "A::B", "A"); + ExpectedParentB.Children.Namespaces.push_back(RB); CheckNamespaceInfo(&ExpectedParentB, ParentB); } diff --git a/clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp b/clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp index ecc2c970f18bc..5382265b18ab4 100644 --- a/clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp +++ b/clang-tools-extra/unittests/clang-doc/YAMLGeneratorTest.cpp @@ -30,9 +30,10 @@ TEST_F(YAMLGeneratorTest, emitNamespaceYAML) { I.Path = "path/to/A"; I.Namespace.emplace_back(EmptySID, "A", InfoType::IT_namespace); - I.Children.Namespaces.emplace_back( - EmptySID, "ChildNamespace", InfoType::IT_namespace, - "path::to::A::Namespace::ChildNamespace", "path/to/A/Namespace"); + Reference NewNamespace(EmptySID, "ChildNamespace", InfoType::IT_namespace, + "path::to::A::Namespace::ChildNamespace", + "path/to/A/Namespace"); + I.Children.Namespaces.push_back(NewNamespace); I.Children.Records.emplace_back(EmptySID, "ChildStruct", InfoType::IT_record, "path::to::A::Namespace::ChildStruct", "path/to/A/Namespace"); _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
