https://github.com/ilovepi updated https://github.com/llvm/llvm-project/pull/190048
>From d5bead9ae8cfcba74bb7ba218b0ddaa6b7f43d96 Mon Sep 17 00:00:00 2001 From: Paul Kirth <[email protected]> Date: Sat, 21 Mar 2026 01:48:26 +0000 Subject: [PATCH] [clang-doc] Migrate Namespaces to arena allocation 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% | --- clang-tools-extra/clang-doc/BitcodeReader.cpp | 6 +++-- clang-tools-extra/clang-doc/JSONGenerator.cpp | 9 ++++--- .../clang-doc/Representation.cpp | 27 ++++++++++++++++++- clang-tools-extra/clang-doc/Representation.h | 10 +++++-- clang-tools-extra/clang-doc/Serialize.cpp | 6 +++-- clang-tools-extra/clang-doc/YAMLGenerator.cpp | 6 +++-- .../unittests/clang-doc/BitcodeTest.cpp | 4 +-- .../unittests/clang-doc/ClangDocTest.cpp | 10 ++++--- .../unittests/clang-doc/JSONGeneratorTest.cpp | 7 ++--- .../unittests/clang-doc/MDGeneratorTest.cpp | 4 +-- .../unittests/clang-doc/MergeTest.cpp | 16 +++++------ .../unittests/clang-doc/SerializeTest.cpp | 8 +++--- .../unittests/clang-doc/YAMLGeneratorTest.cpp | 7 ++--- 13 files changed, 83 insertions(+), 37 deletions(-) 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..e9093a5c8197f 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,15 @@ ClangDocContext::ClangDocContext(tooling::ExecutionContext *ECtx, } void ScopeChildren::sort() { - llvm::sort(Namespaces); + std::vector<Reference *> V; + while (!Namespaces.empty()) { + V.push_back(&Namespaces.front()); + Namespaces.pop_front(); + } + llvm::sort(V, [](const Reference *A, const Reference *B) { return *A < *B; }); + for (auto *R : V) + Namespaces.push_back(*R); + 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
