[clang] Recover performance loss after PagedVector introduction (PR #67972)

Giulio Eulisse via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 2 08:36:44 PDT 2023


https://github.com/ktf updated https://github.com/llvm/llvm-project/pull/67972

>From 154f82bd0e35e9d8ad8f8812ba3eb1cf8d211003 Mon Sep 17 00:00:00 2001
From: Giulio Eulisse <10544+ktf at users.noreply.github.com>
Date: Mon, 2 Oct 2023 13:01:14 +0200
Subject: [PATCH 1/3] Hint for branch likelihood

---
 llvm/include/llvm/ADT/PagedVector.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/include/llvm/ADT/PagedVector.h b/llvm/include/llvm/ADT/PagedVector.h
index 667bece6d718385..f8d014c9c84dab6 100644
--- a/llvm/include/llvm/ADT/PagedVector.h
+++ b/llvm/include/llvm/ADT/PagedVector.h
@@ -84,7 +84,7 @@ template <typename T, size_t PageSize = 1024 / sizeof(T)> class PagedVector {
     assert(Index / PageSize < PageToDataPtrs.size());
     T *&PagePtr = PageToDataPtrs[Index / PageSize];
     // If the page was not yet allocated, allocate it.
-    if (!PagePtr) {
+    if (LLVM_UNLIKELY(!PagePtr)) {
       PagePtr = Allocator.getPointer()->template Allocate<T>(PageSize);
       // We need to invoke the default constructor on all the elements of the
       // page.

>From f119cca040b2bebabff8db32394cc4650f5115d5 Mon Sep 17 00:00:00 2001
From: Giulio Eulisse <10544+ktf at users.noreply.github.com>
Date: Mon, 2 Oct 2023 17:19:43 +0200
Subject: [PATCH 2/3] Change PageSize to 32 elements

Moves from 42 elements (automatically calculated) to 32. It should
both reduce memory usage (it does at least in my test) and avoid
the need for a division when indexing the elements.
---
 clang/include/clang/Basic/SourceManager.h | 2 +-
 clang/lib/Basic/SourceManager.cpp         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/include/clang/Basic/SourceManager.h b/clang/include/clang/Basic/SourceManager.h
index c1b24eec2759c71..ac077d1ab350874 100644
--- a/clang/include/clang/Basic/SourceManager.h
+++ b/clang/include/clang/Basic/SourceManager.h
@@ -700,7 +700,7 @@ class SourceManager : public RefCountedBase<SourceManager> {
   ///
   /// Negative FileIDs are indexes into this table. To get from ID to an index,
   /// use (-ID - 2).
-  llvm::PagedVector<SrcMgr::SLocEntry> LoadedSLocEntryTable;
+  llvm::PagedVector<SrcMgr::SLocEntry, 16> LoadedSLocEntryTable;
 
   /// The starting offset of the next local SLocEntry.
   ///
diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp
index 3066cc53dbfd878..a9c6453da94a3ad 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -2102,7 +2102,7 @@ std::pair<bool, bool> SourceManager::isInTheSameTranslationUnit(
     unsigned Offset;
     FileID ParentFID; // Used for breaking ties.
   };
-  llvm::SmallDenseMap<FileID, Entry, 16> LChain;
+  llvm::SmallDenseMap<FileID, Entry, 32> LChain;
 
   FileID Parent;
   do {

>From 684d85f0c889358fddd52ca2a805caaad025fe10 Mon Sep 17 00:00:00 2001
From: Giulio Eulisse <10544+ktf at users.noreply.github.com>
Date: Mon, 2 Oct 2023 17:36:09 +0200
Subject: [PATCH 3/3] fix

---
 clang/include/clang/Basic/SourceManager.h | 2 +-
 clang/lib/Basic/SourceManager.cpp         | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/clang/include/clang/Basic/SourceManager.h b/clang/include/clang/Basic/SourceManager.h
index ac077d1ab350874..1999e1ff4300b4f 100644
--- a/clang/include/clang/Basic/SourceManager.h
+++ b/clang/include/clang/Basic/SourceManager.h
@@ -700,7 +700,7 @@ class SourceManager : public RefCountedBase<SourceManager> {
   ///
   /// Negative FileIDs are indexes into this table. To get from ID to an index,
   /// use (-ID - 2).
-  llvm::PagedVector<SrcMgr::SLocEntry, 16> LoadedSLocEntryTable;
+  llvm::PagedVector<SrcMgr::SLocEntry, 32> LoadedSLocEntryTable;
 
   /// The starting offset of the next local SLocEntry.
   ///
diff --git a/clang/lib/Basic/SourceManager.cpp b/clang/lib/Basic/SourceManager.cpp
index a9c6453da94a3ad..3066cc53dbfd878 100644
--- a/clang/lib/Basic/SourceManager.cpp
+++ b/clang/lib/Basic/SourceManager.cpp
@@ -2102,7 +2102,7 @@ std::pair<bool, bool> SourceManager::isInTheSameTranslationUnit(
     unsigned Offset;
     FileID ParentFID; // Used for breaking ties.
   };
-  llvm::SmallDenseMap<FileID, Entry, 32> LChain;
+  llvm::SmallDenseMap<FileID, Entry, 16> LChain;
 
   FileID Parent;
   do {



More information about the cfe-commits mailing list