Author: Yao Qi Date: 2026-08-17T13:14:00+01:00 New Revision: 797a057bccd4f0e96f2775a55d2e6401e46e0275
URL: https://github.com/llvm/llvm-project/commit/797a057bccd4f0e96f2775a55d2e6401e46e0275 DIFF: https://github.com/llvm/llvm-project/commit/797a057bccd4f0e96f2775a55d2e6401e46e0275.diff LOG: [lldb] Serve MemoryCache::ReadRanges from the L2 cache as well as L1 (#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. Added: Modified: lldb/include/lldb/Target/Memory.h lldb/packages/Python/lldbsuite/test/gdbclientutils.py lldb/source/Target/Memory.cpp lldb/test/API/lang/objc/foundation/TestObjCMethodsNSError.py lldb/unittests/Target/MemoryTest.cpp Removed: ################################################################################ diff --git a/lldb/include/lldb/Target/Memory.h b/lldb/include/lldb/Target/Memory.h index 2b8655e277a29..ad902ea584449 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,17 @@ 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; + + // 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/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..2cb5a920f6c66 100644 --- a/lldb/source/Target/Memory.cpp +++ b/lldb/source/Target/Memory.cpp @@ -138,6 +138,27 @@ 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; + 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. @@ -280,7 +301,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 +311,11 @@ 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 = FindCacheEntry(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 866a498693be5..16d2d7b8de6eb 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 @@ -68,8 +73,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) { _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
