[lldb] Use MemoryCache in Process::ReadRangesFromMemory - #201166
Merged
felipepiovezan merged 3 commits intoJun 4, 2026
Conversation
There are scenarios (especially in the ObjectiveC metadata reading) in which multiple strings are read over and over again, but through different code paths. In order to make that part of the code use MultiMemRead effectively, the memory cache must be integrated into ReadRangesFromMemory before we can migrate the string reading to vectorized version.
|
@llvm/pr-subscribers-lldb Author: Felipe de Azevedo Piovezan (felipepiovezan) ChangesThere are scenarios (especially in the ObjectiveC metadata reading) in which multiple strings are read over and over again, but through different code paths. In order to make that part of the code use MultiMemRead effectively, the memory cache must be integrated into ReadRangesFromMemory before we can migrate the string reading to vectorized version. Full diff: https://github.com/llvm/llvm-project/pull/201166.diff 4 Files Affected:
diff --git a/lldb/include/lldb/Target/Memory.h b/lldb/include/lldb/Target/Memory.h
index 85584f29ec7e7..2b8655e277a29 100644
--- a/lldb/include/lldb/Target/Memory.h
+++ b/lldb/include/lldb/Target/Memory.h
@@ -11,6 +11,8 @@
#include "lldb/Utility/RangeMap.h"
#include "lldb/lldb-private.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallVector.h"
#include <map>
#include <mutex>
#include <vector>
@@ -31,6 +33,13 @@ class MemoryCache {
size_t Read(lldb::addr_t addr, void *dst, size_t dst_len, Status &error);
+ /// Reads multiple memory ranges, serving cache hits from L1 and batching all
+ /// misses through Process::DoReadMemoryRanges. The semantics of the return
+ /// value match Process::ReadMemoryRanges.
+ llvm::SmallVector<llvm::MutableArrayRef<uint8_t>>
+ ReadRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges,
+ llvm::MutableArrayRef<uint8_t> buffer);
+
uint32_t GetMemoryCacheLineSize() const { return m_L2_cache_line_byte_size; }
void AddInvalidRange(lldb::addr_t base_addr, lldb::addr_t byte_size);
@@ -40,6 +49,11 @@ class MemoryCache {
// Allow external sources to populate data into the L1 memory cache
void AddL1CacheData(lldb::addr_t addr, const void *src, size_t src_len);
+ void AddL1CacheData(lldb::addr_t addr, llvm::ArrayRef<uint8_t> src) {
+ if (!src.empty())
+ AddL1CacheData(addr, src.data(), src.size());
+ }
+
void AddL1CacheData(lldb::addr_t addr,
const lldb::DataBufferSP &data_buffer_sp);
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index f68ea3b639e93..8432c326d3281 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -362,6 +362,7 @@ class Process : public std::enable_shared_from_this<Process>,
friend class StopInfo;
friend class Target;
friend class ThreadList;
+ friend class MemoryCache;
public:
/// Broadcaster event bits definitions.
diff --git a/lldb/source/Target/Memory.cpp b/lldb/source/Target/Memory.cpp
index 6c4650f1eb2d7..bb563be7090d9 100644
--- a/lldb/source/Target/Memory.cpp
+++ b/lldb/source/Target/Memory.cpp
@@ -14,6 +14,8 @@
#include "lldb/Utility/RangeMap.h"
#include "lldb/Utility/State.h"
+#include "llvm/ADT/STLExtras.h"
+
#include <cinttypes>
#include <memory>
@@ -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(
+ results_it, end, [](auto result) { return result.data() == nullptr; });
+ assert(results_it != end);
+ *results_it = fetched;
+ }
+
+ return results;
+}
+
AllocatedBlock::AllocatedBlock(lldb::addr_t addr, uint32_t byte_size,
uint32_t permissions, uint32_t chunk_size)
: m_range(addr, byte_size), m_permissions(permissions),
diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp
index e77cb0b0835e1..ac1357f7d00a1 100644
--- a/lldb/source/Target/Process.cpp
+++ b/lldb/source/Target/Process.cpp
@@ -2077,6 +2077,8 @@ Process::ReadMemoryRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges,
for (const Range<lldb::addr_t, size_t> &range : ranges)
fixed_ranges.emplace_back(FixAnyAddress(range.GetRangeBase()),
range.GetByteSize());
+ if (!GetDisableMemoryCache())
+ return m_memory_cache.ReadRanges(fixed_ranges, buffer);
return DoReadMemoryRanges(fixed_ranges, buffer);
}
|
Contributor
Author
|
No tests are breaking with this, but I will check if there is any other kind of testing we can do. |
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
bulbazord
reviewed
Jun 2, 2026
| auto *results_it = results.begin(); | ||
| auto *end = results.end(); | ||
| for (auto fetched : fetched_buffers) { | ||
| results_it = std::find_if( |
Member
There was a problem hiding this comment.
Instead of iterating via find_if, would it be worth keeping a list of indices for missed ranges? Then you could fill them in directly instead of needing to walk the results.
Contributor
Author
There was a problem hiding this comment.
I thought about that, but felt this code was simple enough that we could avoid the extra allocation. I can give it a try if you feel the extra vector would make the code simpler
Member
There was a problem hiding this comment.
I do think it would be simpler. It could look like:
for (auto [fetched_idx, missing_range_idx] : llvm::enumerate(missing_range_indices)) {
results[missing_range_idx] = fetched_buffers[fetched_idx];
}
Ultimately I find both acceptable, so I'll leave this choice up to you.
Contributor
Author
There was a problem hiding this comment.
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();
Contributor
Author
There was a problem hiding this comment.
Please have a look at the top commit for the precise diff!
bulbazord
approved these changes
Jun 2, 2026
| auto *results_it = results.begin(); | ||
| auto *end = results.end(); | ||
| for (auto fetched : fetched_buffers) { | ||
| results_it = std::find_if( |
Member
There was a problem hiding this comment.
I do think it would be simpler. It could look like:
for (auto [fetched_idx, missing_range_idx] : llvm::enumerate(missing_range_indices)) {
results[missing_range_idx] = fetched_buffers[fetched_idx];
}
Ultimately I find both acceptable, so I'll leave this choice up to you.
Contributor
|
PR testing says a test is failing? |
Contributor
Author
Yeah, this is just the testing re-using an address to test something else, and the caching affects it. I just need to update the address to use a different address |
felipepiovezan
force-pushed
the
felipe/cachemultimemread
branch
from
June 3, 2026 07:41
ed90e15 to
4028ab8
Compare
qiyao
added a commit
that referenced
this pull request
Aug 17, 2026
…216318) `MemoryCache::Read` fetches a whole L2 cache line for any read that fits in one, so reading a few bytes caches the line around them. `ReadRanges` probed only L1, and re-fetched ranges that line already held. Callers hit this whenever they read an array's header and then batch the elements that follow it in the same line, as `AppleObjCRuntimeV2::SharedCacheImageHeaders` and `ClassDescriptorV2::method_list_t` both do. #201166 uses MemoryCache in `Process::ReadRangesFromMemory`, but I didn't see why is L1 used only. Add `FindL2CacheEntry`, a lookup that never reads from the inferior, and consult it after L1. When it serves every range in a batch, `ReadRanges` returns without calling `Process::DoReadMemoryRanges`, so no packet is sent. As in the L1 lookup a range spanning two lines is a miss, and a partially read line is used only up to what it holds. Over the region `TestObjCMethodsNSError.test_runtime_types_efficient_memreads` brackets, `MultiMemRead` drops from 190 packets to 107 and the ranges they carry from 7004 to 6411, with the `m`/`x` count unchanged at 856. That test now also requires no read range to be contained in one an earlier packet already read, which counted 593 ranges before this change and none after. `TestReadMemoryRangesUsesL2Cache` covers the lookup directly.
dyung
pushed a commit
to llvmbot/llvm-project
that referenced
this pull request
Aug 20, 2026
…lvm#216318) `MemoryCache::Read` fetches a whole L2 cache line for any read that fits in one, so reading a few bytes caches the line around them. `ReadRanges` probed only L1, and re-fetched ranges that line already held. Callers hit this whenever they read an array's header and then batch the elements that follow it in the same line, as `AppleObjCRuntimeV2::SharedCacheImageHeaders` and `ClassDescriptorV2::method_list_t` both do. llvm#201166 uses MemoryCache in `Process::ReadRangesFromMemory`, but I didn't see why is L1 used only. Add `FindL2CacheEntry`, a lookup that never reads from the inferior, and consult it after L1. When it serves every range in a batch, `ReadRanges` returns without calling `Process::DoReadMemoryRanges`, so no packet is sent. As in the L1 lookup a range spanning two lines is a miss, and a partially read line is used only up to what it holds. Over the region `TestObjCMethodsNSError.test_runtime_types_efficient_memreads` brackets, `MultiMemRead` drops from 190 packets to 107 and the ranges they carry from 7004 to 6411, with the `m`/`x` count unchanged at 856. That test now also requires no read range to be contained in one an earlier packet already read, which counted 593 ranges before this change and none after. `TestReadMemoryRangesUsesL2Cache` covers the lookup directly. (cherry picked from commit 797a057)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
There are scenarios (especially in the ObjectiveC metadata reading) in which multiple strings are read over and over again, but through different code paths. In order to make that part of the code use MultiMemRead effectively, the memory cache must be integrated into ReadRangesFromMemory before we can migrate the string reading to vectorized version.