diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/memory/CachedBytePage.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/memory/CachedBytePage.java index 374638a305..01a87f1d42 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/memory/CachedBytePage.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/gui/memory/CachedBytePage.java @@ -4,9 +4,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -16,49 +16,93 @@ package ghidra.app.plugin.core.debug.gui.memory; import java.nio.ByteBuffer; +import java.util.*; import ghidra.debug.api.tracemgr.DebuggerCoordinates; import ghidra.program.model.address.Address; public class CachedBytePage { - private boolean valid = true; - private DebuggerCoordinates coordinates; - private Address start; - private byte[] page = new byte[4096]; - private ByteBuffer buf = ByteBuffer.wrap(page); + private static final int PAGE_SIZE = 4096; + /** + * There seem to be a handful of layout indices which are routinely gathered: + *
    + *
  1. The visible layout, to paint it.
  2. + *
  3. Layout[0], to get measurements for scrolling amounts.
  4. + *
  5. The layout containing the cursor, for accessibility.
  6. + *
+ *

+ * We include an extra slack entry so that when a new address is accessed, we don't necessarily + * invalidate an entry that is about to be used. Since the entries often are accessed in a + * cycle, the entry about to be used is often the least-recently used. + *

+ * NOTE: We already instantiate a separate cache for previous vs current coordinates, so no need + * to multiply this by 2. + */ + private static final int CACHE_SIZE = 4; + + private static boolean coordsEqualForMemory(DebuggerCoordinates c1, DebuggerCoordinates c2) { + return c1.getTrace() == c2.getTrace() && c1.getViewSnap() == c2.getViewSnap(); + } + + record CacheKey(DebuggerCoordinates coordinates, Address start) { + int computeOffset(DebuggerCoordinates coordinates, Address address) { + if (coordsEqualForMemory(this.coordinates, coordinates)) { + long offset = address.subtract(start); + if (0 <= offset && offset < PAGE_SIZE) { + return (int) offset; + } + } + return -1; + } + } + + record CacheEntry(byte[] page, ByteBuffer buf) { + public CacheEntry(byte[] page) { + this(page, ByteBuffer.wrap(page)); + } + + public CacheEntry() { + this(new byte[PAGE_SIZE]); + } + + CacheKey refresh(DebuggerCoordinates coordinates, Address address) { + buf.clear(); + Address min = address.getAddressSpace().getMinAddress(); + Address start = address.subtractWrap(page.length / 2); + + if (start.compareTo(min) < 0 || start.compareTo(address) > 0) { + start = min; + } + coordinates.getTrace() + .getMemoryManager() + .getViewBytes(coordinates.getViewSnap(), start, buf); + return new CacheKey(coordinates, start); + } + } + + private final SequencedMap map = new LinkedHashMap<>(); public byte getByte(DebuggerCoordinates coordinates, Address address) { - long offset; - if (!valid || this.coordinates == null || !this.coordinates.equals(coordinates) || - start == null || !start.hasSameAddressSpace(address)) { - offset = refresh(coordinates, address); - } - else { - offset = address.subtract(start); - if (offset < 0 || 4096 <= offset) { - offset = refresh(coordinates, address); + for (Map.Entry ent : map.entrySet()) { + int offset = ent.getKey().computeOffset(coordinates, address); + if (offset != -1) { + // LRU logic: Reset the hit entry's age. + map.remove(ent.getKey()); + map.put(ent.getKey(), ent.getValue()); + return ent.getValue().page[offset]; } } - return page[(int) offset]; + + CacheEntry entry = + map.size() >= CACHE_SIZE ? map.pollFirstEntry().getValue() : new CacheEntry(); + CacheKey key = entry.refresh(coordinates, address); + int offset = key.computeOffset(coordinates, address); + assert offset != -1; + map.put(key, entry); + return entry.page[offset]; } public void invalidate() { - valid = false; - } - - private long refresh(DebuggerCoordinates coordinates, Address address) { - valid = false; - buf.clear(); - Address min = address.getAddressSpace().getMinAddress(); - start = address.subtractWrap(page.length / 2); - - if (start.compareTo(min) < 0 || start.compareTo(address) > 0) { - start = min; - } - coordinates.getTrace() - .getMemoryManager() - .getViewBytes(coordinates.getViewSnap(), start, buf); - valid = true; - return address.subtract(start); + map.clear(); } } diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/memory/DBTraceMemorySpace.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/memory/DBTraceMemorySpace.java index 88c3107cf7..464ce82666 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/memory/DBTraceMemorySpace.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/memory/DBTraceMemorySpace.java @@ -336,6 +336,10 @@ public class DBTraceMemorySpace }*/ } + public void checkStateMapIntegrity() { + stateMapSpace.checkIntegrity(); + } + @Override // TODO: Ensure a code unit is not having rug taken out from under it? public void setState(long snap, Address start, Address end, TraceMemoryState state) { @@ -757,7 +761,8 @@ public class DBTraceMemorySpace spans: for (Lifespan span : viewport.getOrderedSpans(snap)) { Iterator arit = - getAddressesWithState(span, s -> s == TraceMemoryState.KNOWN).iterator(start, true); + getAddressesWithState(span, remains, s -> s == TraceMemoryState.KNOWN) + .iterator(start, true); while (arit.hasNext()) { AddressRange rng = arit.next(); if (rng.getMinAddress().compareTo(toRead.getMaxAddress()) > 0) { diff --git a/Ghidra/Test/DebuggerIntegrationTest/src/test/java/ghidra/app/plugin/core/debug/gui/memory/DebuggerMemoryBytesProviderTest.java b/Ghidra/Test/DebuggerIntegrationTest/src/test/java/ghidra/app/plugin/core/debug/gui/memory/DebuggerMemoryBytesProviderTest.java index 6aa751402f..980476c3b6 100644 --- a/Ghidra/Test/DebuggerIntegrationTest/src/test/java/ghidra/app/plugin/core/debug/gui/memory/DebuggerMemoryBytesProviderTest.java +++ b/Ghidra/Test/DebuggerIntegrationTest/src/test/java/ghidra/app/plugin/core/debug/gui/memory/DebuggerMemoryBytesProviderTest.java @@ -17,6 +17,7 @@ package ghidra.app.plugin.core.debug.gui.memory; import static ghidra.lifecycle.Unfinished.TODO; import static org.junit.Assert.*; +import static org.junit.Assume.assumeFalse; import java.awt.*; import java.awt.datatransfer.Clipboard; @@ -62,13 +63,16 @@ import ghidra.program.model.lang.RegisterValue; import ghidra.program.util.ProgramLocation; import ghidra.trace.database.ToyDBTraceBuilder; import ghidra.trace.database.memory.DBTraceMemoryManager; +import ghidra.trace.database.memory.DBTraceMemorySpace; import ghidra.trace.database.stack.DBTraceStackManager; +import ghidra.trace.database.time.DBTraceTimeManager; import ghidra.trace.model.Lifespan; import ghidra.trace.model.memory.*; import ghidra.trace.model.modules.TraceModule; import ghidra.trace.model.stack.TraceStack; import ghidra.trace.model.target.TraceObject; import ghidra.trace.model.thread.TraceThread; +import ghidra.util.SystemUtilities; @Category(NightlyCategory.class) public class DebuggerMemoryBytesProviderTest extends AbstractGhidraHeadedDebuggerIntegrationTest { @@ -1184,4 +1188,33 @@ public class DebuggerMemoryBytesProviderTest extends AbstractGhidraHeadedDebugge rmiCx.withdrawTarget(tool, tb.trace); } + + @Test + public void testPerformanceManuallyWithManyManySnaps() throws Exception { + assumeFalse(SystemUtilities.isInTestingBatchMode()); + createAndOpenTrace(); + + // LATER (GP-5594): 100_000 without checkStateMapIntegrity will crash. + final long snapCount = 100_000; + try (Transaction tx = tb.startTransaction()) { + tb.trace.getMemoryManager() + .addRegion("Processes[1].Memory[exe:.text]", Lifespan.nowOn(0L), + tb.range(0x55550000, 0x5555ffff), TraceMemoryFlag.READ, + TraceMemoryFlag.EXECUTE); + DBTraceTimeManager time = tb.trace.getTimeManager(); + DBTraceMemorySpace space = tb.trace.getMemoryManager() + .getForSpace(tb.host.getAddressFactory().getDefaultAddressSpace(), true); + for (int i = 0; i < snapCount; i++) { + time.getSnapshot(i, true); + space.putBytes(i, tb.addr(0x55550000 + (i & 0xffff)), tb.buf(i & 0xff)); + if (i % 1000 == 0) { + space.checkStateMapIntegrity(); + } + } + } + traceManager.activateTrace(tb.trace); + traceManager.activateSnap(snapCount - 1); + + Thread.sleep(1); // breakpoint here + } }