https://github.com/qiyao updated https://github.com/llvm/llvm-project/pull/216318
>From 1ed8b78415db80ebb3cc4ef39fae5fb405908076 Mon Sep 17 00:00:00 2001 From: Yao Qi <[email protected]> Date: Thu, 13 Aug 2026 23:23:04 +0100 Subject: [PATCH 1/2] [lldb] Serve MemoryCache::ReadRanges from the L2 cache as well as L1 `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. --- lldb/include/lldb/Target/Memory.h | 12 +- .../Python/lldbsuite/test/gdbclientutils.py | 27 +++++ lldb/source/Target/Memory.cpp | 23 +++- .../objc/foundation/TestObjCMethodsNSError.py | 33 +++++- lldb/unittests/Target/MemoryTest.cpp | 111 ++++++++++++++++++ 5 files changed, 198 insertions(+), 8 deletions(-) diff --git a/lldb/include/lldb/Target/Memory.h b/lldb/include/lldb/Target/Memory.h index 2b8655e277a29..1f3cba224e5bf 100644 --- a/lldb/include/lldb/Target/Memory.h +++ b/lldb/include/lldb/Target/Memory.h @@ -33,9 +33,9 @@ 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. + /// Reads multiple memory ranges, serving cache hits from L1 and L2 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); @@ -82,6 +82,12 @@ class MemoryCache { // returns a pointer into that entry's data at the correct offset. Returns // nullptr on a miss. Caller must hold m_mutex. const uint8_t *FindL1CacheEntry(lldb::addr_t addr, size_t len) const; + + // If the entire range [addr, addr+len) is covered by a single cache line + // that is already in L2, returns a pointer into that line's data at the + // correct offset. Never reads from the inferior; returns nullptr on a miss. + // Caller must hold m_mutex. + const uint8_t *FindL2CacheEntry(lldb::addr_t addr, size_t len) const; }; diff --git a/lldb/packages/Python/lldbsuite/test/gdbclientutils.py b/lldb/packages/Python/lldbsuite/test/gdbclientutils.py index afd0b12b20379..abe1da925cb40 100644 --- a/lldb/packages/Python/lldbsuite/test/gdbclientutils.py +++ b/lldb/packages/Python/lldbsuite/test/gdbclientutils.py @@ -95,6 +95,33 @@ def parse_memory_read_packet(packet): return addr, length +def parse_memory_read_ranges(packet: str) -> List[Tuple[int, int]]: + """ + Parse every (addr, length) a memory-read packet asks the stub for, and return + an empty list if the packet isn't a memory read. Unlike + parse_memory_read_packet this also covers "MultiMemRead", which carries + several ranges in one packet. + """ + single = parse_memory_read_packet(packet) + if single is not None: + return [single] + + prefix = "MultiMemRead:ranges:" + if not packet or not packet.startswith(prefix): + return [] + body = packet[len(prefix) :] + end = body.find(";") + if end < 0: + return [] + try: + numbers = [int(n, 16) for n in body[:end].split(",")] + except ValueError: + return [] + if len(numbers) % 2: + return [] + return list(zip(numbers[0::2], numbers[1::2])) + + class PacketDirection(Enum): RECV = "recv" SEND = "send" diff --git a/lldb/source/Target/Memory.cpp b/lldb/source/Target/Memory.cpp index 6e372fdf3fbee..b8749bfd95868 100644 --- a/lldb/source/Target/Memory.cpp +++ b/lldb/source/Target/Memory.cpp @@ -138,6 +138,20 @@ const uint8_t *MemoryCache::FindL1CacheEntry(lldb::addr_t addr, return pos->second->GetBytes() + (addr - chunk_range.GetRangeBase()); } +const uint8_t *MemoryCache::FindL2CacheEntry(lldb::addr_t addr, + size_t len) const { + if (m_L2_cache.empty()) + return nullptr; + const lldb::addr_t line_offset = addr % m_L2_cache_line_byte_size; + BlockMap::const_iterator pos = m_L2_cache.find(addr - line_offset); + if (pos == m_L2_cache.end()) + return nullptr; + // Like the L1 lookup, a read spanning two lines is treated as a miss. + if (line_offset + len > pos->second->GetByteSize()) + return nullptr; + return pos->second->GetBytes() + line_offset; +} + lldb::DataBufferSP MemoryCache::GetL2CacheLine(lldb::addr_t line_base_addr, Status &error) { // This function assumes that the address given is aligned correctly. @@ -280,7 +294,7 @@ MemoryCache::ReadRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges, results.reserve(ranges.size()); llvm::SmallVector<Range<lldb::addr_t, size_t>> missed_ranges; - // Iterate once serving requests from L1. + // Iterate once serving requests from the caches. for (auto range : ranges) { const lldb::addr_t addr = range.GetRangeBase(); const size_t len = range.GetByteSize(); @@ -290,10 +304,13 @@ MemoryCache::ReadRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges, continue; } - if (const uint8_t *l1_data = FindL1CacheEntry(addr, len)) { + const uint8_t *cached = FindL1CacheEntry(addr, len); + if (!cached) + cached = FindL2CacheEntry(addr, len); + if (cached) { results.push_back(buffer.take_front(len)); buffer = buffer.drop_front(len); - memcpy(results.back().data(), l1_data, len); + memcpy(results.back().data(), cached, len); continue; } diff --git a/lldb/test/API/lang/objc/foundation/TestObjCMethodsNSError.py b/lldb/test/API/lang/objc/foundation/TestObjCMethodsNSError.py index a9fbe54e074ff..a72b04850d045 100644 --- a/lldb/test/API/lang/objc/foundation/TestObjCMethodsNSError.py +++ b/lldb/test/API/lang/objc/foundation/TestObjCMethodsNSError.py @@ -5,6 +5,11 @@ import lldb from lldbsuite.test.decorators import * +from lldbsuite.test.gdbclientutils import ( + PacketDirection, + parse_memory_read_ranges, + parse_packet_log, +) from lldbsuite.test.lldbtest import * from lldbsuite.test import lldbutil @@ -67,8 +72,32 @@ def test_runtime_types_efficient_memreads(self): log_text = open(logfile).read() log_text = log_text.split("StartTesting", 1)[-1].split("EndTesting", 1)[0] - # This test is only checking that the packet it used at all (and that - # no errors are produced). It doesn't check that the packet is being + # The two assertions below only check that the packet is used at all (and + # that no errors are produced). They don't check that the packet is being # used to solve a problem in an optimal way. self.assertIn("MultiMemRead:", log_text) self.assertNotIn("MultiMemRead error", log_text) + + # The memory cache serves a read out of what an earlier read fetched, so + # no range may be contained in one an earlier packet already read. + requested = [] + for direction, body in parse_packet_log(log_text.splitlines()): + if direction != PacketDirection.SEND: + continue + + # Resuming drops the whole cache, so nothing read before it is still + # held. + if body[0] in "cCsS" or body.startswith("vCont;"): + requested.clear() + continue + + ranges = parse_memory_read_ranges(body) + for addr, length in ranges: + self.assertFalse( + any( + addr >= base and addr + length <= base + size + for base, size in requested + ), + f"re-read {length} bytes at {addr:#x}", + ) + requested.extend(ranges) diff --git a/lldb/unittests/Target/MemoryTest.cpp b/lldb/unittests/Target/MemoryTest.cpp index f9d3dc1fee72f..8035b4b4d5c82 100644 --- a/lldb/unittests/Target/MemoryTest.cpp +++ b/lldb/unittests/Target/MemoryTest.cpp @@ -542,6 +542,117 @@ TEST_F(MemoryTest, TestReadMemoryRanges) { } } +TEST_F(MemoryTest, TestReadMemoryRangesUsesL2Cache) { + ArchSpec arch("x86_64-apple-macosx-"); + + Platform::SetHostPlatform(PlatformRemoteMacOSX::CreateInstance(true, &arch)); + + DebuggerSP debugger_sp = Debugger::CreateInstance(); + ASSERT_TRUE(debugger_sp); + + TargetSP target_sp = CreateTarget(debugger_sp, arch); + ASSERT_TRUE(target_sp); + + ProcessSP process_sp = CreateProcess(target_sp); + ASSERT_TRUE(process_sp); + + DummyProcess *process = static_cast<DummyProcess *>(process_sp.get()); + const uint64_t l2_cache_size = process->GetMemoryCacheLineSize(); + Status error; + uint8_t header[8]; + + // Read the first 8 bytes of a cache line, the way a caller reads the header + // of an array before batching the elements that follow it. This fills the + // whole line and leaves the inferior unable to supply anything more. + const addr_t full_line = 0x1000; + ASSERT_EQ(full_line % l2_cache_size, 0u); + process->SetMaxReadSize(l2_cache_size); + process->SetFiller('A'); + ASSERT_EQ(process->ReadMemory(full_line, header, sizeof(header), error), + sizeof(header)); + ASSERT_EQ(process->m_bytes_left, 0u); + + { // Ranges covered by that line are served from the cache. Leave the inferior + // able to answer, with a filler of its own, so a miss would show up both in + // the contents below and in the unspent budget. + process->SetMaxReadSize(l2_cache_size); + process->SetFiller('X'); + llvm::SmallVector<uint8_t, 0> buffer(3 * 8, 0); + llvm::SmallVector<Range<addr_t, size_t>> ranges = { + {full_line + 8, 8}, + {full_line + 16, 8}, + {full_line + l2_cache_size - 8, 8}}; + llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> read_results = + process->ReadMemoryRanges(ranges, buffer); + ASSERT_EQ(read_results.size(), ranges.size()); + for (llvm::MutableArrayRef<uint8_t> memory : read_results) { + ASSERT_EQ(memory.size(), 8u); + for (uint8_t byte : memory) + EXPECT_EQ(byte, 'A'); + } + // Nothing was read from the inferior, so no packet was sent. + EXPECT_EQ(process->m_bytes_left, l2_cache_size); + } + + { // A range crossing into the next, uncached line is a miss. + process->SetMaxReadSize(0); + llvm::SmallVector<uint8_t, 0> buffer(8, 0); + llvm::SmallVector<Range<addr_t, size_t>> ranges = { + {full_line + l2_cache_size - 4, 8}}; + llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> read_results = + process->ReadMemoryRanges(ranges, buffer); + ASSERT_EQ(read_results.size(), 1u); + EXPECT_EQ(read_results[0].size(), 0u); + } + + { // A batch of hits and misses keeps the results in the requested order, and + // asks the inferior for the missed range only. + const addr_t uncached_line = 0x3000; + process->SetMaxReadSize(l2_cache_size); + process->SetFiller('C'); + llvm::SmallVector<uint8_t, 0> buffer(3 * 8, 0); + llvm::SmallVector<Range<addr_t, size_t>> ranges = { + {full_line + 8, 8}, {uncached_line, 8}, {full_line + 16, 8}}; + llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> read_results = + process->ReadMemoryRanges(ranges, buffer); + ASSERT_EQ(read_results.size(), ranges.size()); + for (llvm::MutableArrayRef<uint8_t> memory : read_results) + ASSERT_EQ(memory.size(), 8u); + for (uint8_t byte : read_results[0]) + EXPECT_EQ(byte, 'A'); + for (uint8_t byte : read_results[1]) + EXPECT_EQ(byte, 'C'); + for (uint8_t byte : read_results[2]) + EXPECT_EQ(byte, 'A'); + EXPECT_EQ(process->m_bytes_left, l2_cache_size - 8); + } + + // A line the inferior could only partially supply is cached short. + const addr_t short_line = 0x2000; + ASSERT_EQ(short_line % l2_cache_size, 0u); + const size_t bytes_available = 64; + ASSERT_LT(bytes_available, l2_cache_size); + process->SetMaxReadSize(bytes_available); + process->SetFiller('D'); + ASSERT_EQ(process->ReadMemory(short_line, header, sizeof(header), error), + sizeof(header)); + ASSERT_EQ(process->m_bytes_left, 0u); + + { // Only the part of the line that was actually read may be served. + llvm::SmallVector<uint8_t, 0> buffer(2 * 8, 0); + llvm::SmallVector<Range<addr_t, size_t>> ranges = { + {short_line + bytes_available - 8, 8}, + {short_line + bytes_available - 4, 8}}; + llvm::SmallVector<llvm::MutableArrayRef<uint8_t>> read_results = + process->ReadMemoryRanges(ranges, buffer); + ASSERT_EQ(read_results.size(), ranges.size()); + ASSERT_EQ(read_results[0].size(), 8u); + for (uint8_t byte : read_results[0]) + EXPECT_EQ(byte, 'D'); + EXPECT_EQ(read_results[1].size(), 0u); + } +} + using MemoryDeathTest = MemoryTest; TEST_F(MemoryDeathTest, TestReadMemoryRangesReturnsTooMuch) { >From bcf0d13cbab6766ee894d2aa6f90bdb86bba6168 Mon Sep 17 00:00:00 2001 From: Yao Qi <[email protected]> Date: Mon, 17 Aug 2026 12:44:33 +0100 Subject: [PATCH 2/2] Add FindCacheEntry --- lldb/include/lldb/Target/Memory.h | 7 ++++++- lldb/source/Target/Memory.cpp | 13 +++++++++---- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/lldb/include/lldb/Target/Memory.h b/lldb/include/lldb/Target/Memory.h index 1f3cba224e5bf..ad902ea584449 100644 --- a/lldb/include/lldb/Target/Memory.h +++ b/lldb/include/lldb/Target/Memory.h @@ -85,9 +85,14 @@ class MemoryCache { // If the entire range [addr, addr+len) is covered by a single cache line // that is already in L2, returns a pointer into that line's data at the - // correct offset. Never reads from the inferior; returns nullptr on a miss. + // correct offset. Never reads from the inferior. Returns nullptr on a miss. // Caller must hold m_mutex. const uint8_t *FindL2CacheEntry(lldb::addr_t addr, size_t len) const; + + // Looks the range [addr, addr+len) up in L1 and then L2, returning a pointer + // into the data of whichever entry covers it. Returns nullptr on a miss. + // Caller must hold m_mutex. + const uint8_t *FindCacheEntry(lldb::addr_t addr, size_t len) const; }; diff --git a/lldb/source/Target/Memory.cpp b/lldb/source/Target/Memory.cpp index b8749bfd95868..2cb5a920f6c66 100644 --- a/lldb/source/Target/Memory.cpp +++ b/lldb/source/Target/Memory.cpp @@ -146,12 +146,19 @@ const uint8_t *MemoryCache::FindL2CacheEntry(lldb::addr_t addr, BlockMap::const_iterator pos = m_L2_cache.find(addr - line_offset); if (pos == m_L2_cache.end()) return nullptr; - // Like the L1 lookup, a read spanning two lines is treated as a miss. if (line_offset + len > pos->second->GetByteSize()) return nullptr; return pos->second->GetBytes() + line_offset; } +const uint8_t *MemoryCache::FindCacheEntry(lldb::addr_t addr, + size_t len) const { + const uint8_t *cached = FindL1CacheEntry(addr, len); + if (!cached) + cached = FindL2CacheEntry(addr, len); + return cached; +} + lldb::DataBufferSP MemoryCache::GetL2CacheLine(lldb::addr_t line_base_addr, Status &error) { // This function assumes that the address given is aligned correctly. @@ -304,9 +311,7 @@ MemoryCache::ReadRanges(llvm::ArrayRef<Range<lldb::addr_t, size_t>> ranges, continue; } - const uint8_t *cached = FindL1CacheEntry(addr, len); - if (!cached) - cached = FindL2CacheEntry(addr, len); + const uint8_t *cached = FindCacheEntry(addr, len); if (cached) { results.push_back(buffer.take_front(len)); buffer = buffer.drop_front(len); _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
