[Lldb-commits] [lldb] [lldb] Use batched memory reads in ClassDescriptorV2::relative_list_entry_t (PR #201284)

[via lldb-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->

@llvm/pr-subscribers-lldb

Author: Felipe de Azevedo Piovezan (felipepiovezan)

<details>
<summary>Changes</summary>

This reduces the number of memory reads performed when reading Objective C classes metadata.
Note: these addresses are indeed sequential (with a small offset between them), but there are so many of them that they would not fit into a single Process::ReadMemory cache line, so this is still a win.

---
Full diff: https://github.com/llvm/llvm-project/pull/201284.diff


2 Files Affected:

- (modified) lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp (+34-31) 
- (modified) lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h (+4-3) 


``````````diff
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
index 38481ff9e6cfd..1b8992d5e3f5d 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.cpp
@@ -393,29 +393,27 @@ ClassDescriptorV2::ivar_t::Read(Process *process, lldb::addr_t addr) {
   return result;
 }
 
-llvm::Expected<ClassDescriptorV2::relative_list_entry_t>
-ClassDescriptorV2::relative_list_entry_t::Read(Process *process,
-                                               lldb::addr_t addr) {
+llvm::Expected<llvm::SmallVector<ClassDescriptorV2::relative_list_entry_t>>
+ClassDescriptorV2::ReadRelativeListEntries(Process &process,
+                                           llvm::ArrayRef<lldb::addr_t> addrs) {
   size_t size = sizeof(uint64_t); // m_image_index : 16
                                   // m_list_offset : 48
 
-  DataBufferHeap buffer(size, '\0');
-  Status error;
-
-  process->ReadMemory(addr, buffer.GetBytes(), size, error);
-  if (error.Fail())
-    return llvm::joinErrors(
-        error.takeError(),
-        llvm::createStringErrorV(
-            "Failed to read relative_list_entry_t at address {0:x}", addr));
-
-  DataExtractor extractor(buffer.GetBytes(), size, process->GetByteOrder(),
-                          process->GetAddressByteSize());
-  lldb::offset_t cursor = 0;
-  uint64_t raw_entry = extractor.GetU64_unchecked(&cursor);
-  uint16_t image_index = raw_entry & 0xFFFF;
-  int64_t list_offset = llvm::SignExtend64<48>(raw_entry >> 16);
-  return relative_list_entry_t{image_index, list_offset};
+  llvm::SmallVector<std::optional<uint64_t>> raw_entries =
+      process.ReadUnsignedIntegersFromMemory(addrs, size);
+
+  llvm::SmallVector<relative_list_entry_t> results;
+  results.reserve(addrs.size());
+  for (auto [addr, maybe_raw] : llvm::zip(addrs, raw_entries)) {
+    if (!maybe_raw)
+      return llvm::createStringErrorV(
+          "Failed to read relative_list_entry_t at address {0:x}", addr);
+    uint64_t raw = *maybe_raw;
+    uint16_t image_index = raw & 0xFFFF;
+    int64_t list_offset = llvm::SignExtend64<48>(raw >> 16);
+    results.push_back(relative_list_entry_t{image_index, list_offset});
+  }
+  return results;
 }
 
 llvm::Expected<ClassDescriptorV2::relative_list_list_t>
@@ -505,18 +503,23 @@ llvm::Error ClassDescriptorV2::ProcessRelativeMethodLists(
   if (!relative_method_lists)
     return relative_method_lists.takeError();
 
-  for (uint32_t i = 0; i < relative_method_lists->m_count; i++) {
-    // 2. Extract the image index and the list offset from the
-    // relative_list_entry_t
-    const lldb::addr_t entry_addr = relative_method_lists->m_first_ptr +
-                                    (i * relative_method_lists->m_entsize);
-    auto entry = relative_list_entry_t::Read(process, entry_addr);
-    if (!entry)
-      return entry.takeError();
+  // 2. Compute the address of every relative_list_entry_t and read them all in
+  // a single batched memory read.
+  auto to_entry_addr = [&](uint64_t idx) {
+    return relative_method_lists->m_first_ptr +
+           (idx * relative_method_lists->m_entsize);
+  };
+  auto entry_addrs = llvm::to_vector(llvm::map_range(
+      llvm::seq<uint64_t>(relative_method_lists->m_count), to_entry_addr));
+
+  auto entries = ReadRelativeListEntries(*process, entry_addrs);
+  if (!entries)
+    return entries.takeError();
 
+  for (auto [entry_addr, entry] : llvm::zip(entry_addrs, *entries)) {
     // 3. Calculate the pointer to the method_list_t from the
     // relative_list_entry_t
-    const lldb::addr_t method_list_addr = entry_addr + entry->m_list_offset;
+    const lldb::addr_t method_list_addr = entry_addr + entry.m_list_offset;
 
     // 4. Get the method_list_t from the pointer
     llvm::Expected<method_list_t> method_list =
@@ -525,10 +528,10 @@ llvm::Error ClassDescriptorV2::ProcessRelativeMethodLists(
       return method_list.takeError();
 
     // 5. Cache the result so we don't need to reconstruct it later.
-    m_image_to_method_lists[entry->m_image_index].emplace_back(*method_list);
+    m_image_to_method_lists[entry.m_image_index].emplace_back(*method_list);
 
     // 6. If the relevant image is loaded, add the methods to the Decl
-    if (!m_runtime.IsSharedCacheImageLoaded(entry->m_image_index))
+    if (!m_runtime.IsSharedCacheImageLoaded(entry.m_image_index))
       continue;
 
     ProcessMethodList(instance_method_func, *method_list);
diff --git a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
index e861203e7f7d1..e62eedf92c579 100644
--- a/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
+++ b/lldb/source/Plugins/LanguageRuntime/ObjC/AppleObjCRuntime/AppleObjCClassDescriptorV2.h
@@ -210,11 +210,12 @@ class ClassDescriptorV2 : public ObjCLanguageRuntime::ClassDescriptor {
   struct relative_list_entry_t {
     uint16_t m_image_index;
     int64_t m_list_offset;
-
-    static llvm::Expected<relative_list_entry_t> Read(Process *process,
-                                                      lldb::addr_t addr);
   };
 
+  static llvm::Expected<
+      llvm::SmallVector<ClassDescriptorV2::relative_list_entry_t>>
+  ReadRelativeListEntries(Process &process, llvm::ArrayRef<lldb::addr_t> addrs);
+
   struct relative_list_list_t {
     uint32_t m_entsize;
     uint32_t m_count;

``````````

</details>


https://github.com/llvm/llvm-project/pull/201284