[Lldb-commits] [lldb] [lldb] Use MemoryCache in Process::ReadRangesFromMemory (PR #201166)

[Felipe de Azevedo Piovezan via lldb-commits]

================
@@ -270,6 +272,57 @@ size_t MemoryCache::Read(addr_t addr, void *dst, size_t dst_len,
   return dst_len;
 }
 
+llvm::SmallVector<llvm::MutableArrayRef<uint8_t>>
+MemoryCache::ReadRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges,
+                        llvm::MutableArrayRef<uint8_t> buffer) {
+  std::lock_guard<std::recursive_mutex> guard(m_mutex);
+
+  llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> results;
+  results.reserve(ranges.size());
+  llvm::SmallVector<Range<lldb::addr_t, size_t>> missed_ranges;
+
+  // Iterate once serving requests from L1.
+  for (auto range : ranges) {
+    const lldb::addr_t addr = range.GetRangeBase();
+    const size_t len = range.GetByteSize();
+
+    if (m_invalid_ranges.FindEntryThatContains(addr)) {
+      results.push_back(buffer.take_front(0));
+      continue;
+    }
+
+    if (const uint8_t *l1_data = FindL1CacheEntry(addr, len)) {
+      results.push_back(buffer.take_front(len));
+      buffer = buffer.drop_front(len);
+      memcpy(results.back().data(), l1_data, len);
+      continue;
+    }
+
+    // Use a nullptr to denote this needs fetching.
+    results.emplace_back(nullptr, nullptr);
+    missed_ranges.push_back(range);
+  }
+
+  if (missed_ranges.empty())
+    return results;
+
+  auto fetched_buffers = m_process.DoReadMemoryRanges(missed_ranges, buffer);
+
+  for (auto [missed_range, fetched] : llvm::zip(missed_ranges, fetched_buffers))
+    AddL1CacheData(missed_range.GetRangeBase(), fetched);
+
+  auto *results_it = results.begin();
+  auto *end = results.end();
+  for (auto fetched : fetched_buffers) {
+    results_it = std::find_if(
----------------
felipepiovezan wrote:

I gave this a try, but I was not the biggest fan of how this makes the previous loop a bit more complicated (have to `enumerate` the array just to have an index that is used at the very end of the loop, plus the extra vector).

Instead, I think I found a middle ground that removes the `find_if`, and removes the need for an extra vector:

```
  for (auto &result : results)
    if (result.data() == nullptr)
      result = fetched_buffers.consume_front();
```

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