[llvm] [DebugNames] Compare TableEntry names more efficiently (PR #79759)

[Felipe de Azevedo Piovezan via llvm-commits]

https://github.com/felipepiovezan updated https://github.com/llvm/llvm-project/pull/79759

>From d5ca5971ef441e6c38d0656e6fd59463d3d1a4bf Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Tue, 23 Jan 2024 16:26:34 -0800
Subject: [PATCH 1/4] [DebugNames] Compare TableEntry names more efficiently

TableEntry names are pointers into the string table section, and accessing their
length requires a search for `\0`. However, 99% of the time we only need to
compare the name against some other other, and such a comparison will fail as
early as the first character.

This commit adds a method to the interface of TableEntry so that such a
comparison can be done without extracting the full name. It saves 10% in the
time (1250ms -> 1100 ms) to evaluate the following expression.

```
lldb \
  --batch \
  -o "b CodeGenFunction::GenerateCode" \
  -o run \
  -o "expr Fn" \
  -- \
  clang++ -c -g test.cpp -o /dev/null &> output
```
---
 .../llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h    | 13 +++++++++++++
 llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp  |  4 ++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
index 44a19c7b13f9a..305a8f4220812 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
@@ -543,6 +543,19 @@ class DWARFDebugNames : public DWARFAcceleratorTable {
       return StrData.getCStr(&Off);
     }
 
+    /// Compares the name of this entry against Target, returning true if they
+    /// are equal. This is helpful is hot code paths that do not need the length
+    /// of the name.
+    bool sameNameAs(StringRef Target) const {
+      // Note: this is not the name, but the rest of debug_names starting from
+      // name. This handles corrupt data (non-null terminated) without
+      // overrunning the buffer.
+      auto Data = StrData.getData().substr(StringOffset);
+      auto TargetSize = Target.size();
+      return Data.size() > TargetSize && !Data[TargetSize] &&
+             strncmp(Data.data(), Target.data(), TargetSize) == 0;
+    }
+
     /// Returns the offset of the first Entry in the list.
     uint64_t getEntryOffset() const { return EntryOffset; }
   };
diff --git a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
index 03ad5d133cadd..94d52b6c4ade5 100644
--- a/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
+++ b/llvm/lib/DebugInfo/DWARF/DWARFAcceleratorTable.cpp
@@ -921,7 +921,7 @@ DWARFDebugNames::ValueIterator::findEntryOffsetInCurrentIndex() {
   if (Hdr.BucketCount == 0) {
     // No Hash Table, We need to search through all names in the Name Index.
     for (const NameTableEntry &NTE : *CurrentIndex) {
-      if (NTE.getString() == Key)
+      if (NTE.sameNameAs(Key))
         return NTE.getEntryOffset();
     }
     return std::nullopt;
@@ -942,7 +942,7 @@ DWARFDebugNames::ValueIterator::findEntryOffsetInCurrentIndex() {
       return std::nullopt; // End of bucket
 
     NameTableEntry NTE = CurrentIndex->getNameTableEntry(Index);
-    if (NTE.getString() == Key)
+    if (NTE.sameNameAs(Key))
       return NTE.getEntryOffset();
   }
   return std::nullopt;

>From e73146e2524ac4911a7d00a0af183dbd3f7580eb Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Tue, 30 Jan 2024 12:17:57 -0800
Subject: [PATCH 2/4] fixup! Fix typo in comment

---
 llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
index 305a8f4220812..9cde3178058d4 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
@@ -547,7 +547,7 @@ class DWARFDebugNames : public DWARFAcceleratorTable {
     /// are equal. This is helpful is hot code paths that do not need the length
     /// of the name.
     bool sameNameAs(StringRef Target) const {
-      // Note: this is not the name, but the rest of debug_names starting from
+      // Note: this is not the name, but the rest of debug_str starting from
       // name. This handles corrupt data (non-null terminated) without
       // overrunning the buffer.
       auto Data = StrData.getData().substr(StringOffset);

>From 01ef075e120b771bbafcb18e89643e8c31b4a822 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Tue, 30 Jan 2024 12:25:34 -0800
Subject: [PATCH 3/4] fixup! Updat comment based on review

---
 llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
index 9cde3178058d4..a2d3d56ec5ada 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
@@ -544,8 +544,8 @@ class DWARFDebugNames : public DWARFAcceleratorTable {
     }
 
     /// Compares the name of this entry against Target, returning true if they
-    /// are equal. This is helpful is hot code paths that do not need the length
-    /// of the name.
+    /// are equal. This is more efficient in hot code paths that do not need the
+    /// length of the name.
     bool sameNameAs(StringRef Target) const {
       // Note: this is not the name, but the rest of debug_str starting from
       // name. This handles corrupt data (non-null terminated) without

>From 98e0bfa8fb4b5aed724cf420b5e226d624a1c446 Mon Sep 17 00:00:00 2001
From: Felipe de Azevedo Piovezan <fpiovezan at apple.com>
Date: Tue, 30 Jan 2024 12:43:37 -0800
Subject: [PATCH 4/4] fixup! Update types based on review

---
 llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
index a2d3d56ec5ada..da2f43bfb56c7 100644
--- a/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
+++ b/llvm/include/llvm/DebugInfo/DWARF/DWARFAcceleratorTable.h
@@ -550,8 +550,8 @@ class DWARFDebugNames : public DWARFAcceleratorTable {
       // Note: this is not the name, but the rest of debug_str starting from
       // name. This handles corrupt data (non-null terminated) without
       // overrunning the buffer.
-      auto Data = StrData.getData().substr(StringOffset);
-      auto TargetSize = Target.size();
+      StringRef Data = StrData.getData().substr(StringOffset);
+      size_t TargetSize = Target.size();
       return Data.size() > TargetSize && !Data[TargetSize] &&
              strncmp(Data.data(), Target.data(), TargetSize) == 0;
     }