From e1a186a5d0a3914c8f5676ddf8072fd4813900f2 Mon Sep 17 00:00:00 2001 From: Dan <46821332+nsadeveloper789@users.noreply.github.com> Date: Fri, 23 Sep 2022 15:36:21 -0400 Subject: [PATCH] GP-2593: Cache bytes in DBTraceProgramViewMemory --- .../java/ghidra/trace/database/DBTrace.java | 4 + .../database/memory/DBTraceMemorySpace.java | 9 +- ...AbstractDBTraceProgramViewMemoryBlock.java | 68 +++++--- .../trace/database/program/ByteCache.java | 152 ++++++++++++++++++ .../database/program/DBTraceProgramView.java | 4 + .../program/DBTraceProgramViewMemory.java | 17 ++ 6 files changed, 228 insertions(+), 26 deletions(-) create mode 100644 Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/ByteCache.java diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/DBTrace.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/DBTrace.java index 260a3018cc..bfe846e898 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/DBTrace.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/DBTrace.java @@ -812,4 +812,8 @@ public class DBTrace extends DBCachedDomainObjectAdapter implements Trace, Trace public void updateViewsRefreshBlocks() { allViews(v -> v.updateMemoryRefreshBlocks()); } + + public void updateViewsBytesChanged(AddressRange range) { + allViews(v -> v.updateBytesChanged(range)); + } } 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 cad09e3e9d..b01613edb2 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 @@ -676,16 +676,18 @@ public class DBTraceMemorySpace // Read back the written bytes and fire event byte[] bytes = Arrays.copyOfRange(buf.array(), arrOff, arrOff + result); + ImmutableTraceAddressSnapRange tasr = new ImmutableTraceAddressSnapRange(start, + start.add(result - 1), snap, lastSnap.snap); trace.setChanged(new TraceChangeRecord<>(TraceMemoryBytesChangeType.CHANGED, - this, new ImmutableTraceAddressSnapRange(start, start.add(result - 1), - snap, lastSnap.snap), - oldBytes.array(), bytes)); + this, tasr, oldBytes.array(), bytes)); // Fixup affected code units DBTraceCodeSpace codeSpace = trace.getCodeManager().get(this, false); if (codeSpace != null) { codeSpace.bytesChanged(changed, snap, start, oldBytes.array(), bytes); } + // Clear program view caches + trace.updateViewsBytesChanged(tasr.getRange()); } return result; } @@ -1047,6 +1049,7 @@ public class DBTraceMemorySpace regionMapSpace.invalidateCache(); regionCache.clear(); trace.updateViewsRefreshBlocks(); + trace.updateViewsBytesChanged(null); stateMapSpace.invalidateCache(); bufferStore.invalidateCache(); blockStore.invalidateCache(); diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/AbstractDBTraceProgramViewMemoryBlock.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/AbstractDBTraceProgramViewMemoryBlock.java index ba52f6b091..12ee02e1a9 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/AbstractDBTraceProgramViewMemoryBlock.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/AbstractDBTraceProgramViewMemoryBlock.java @@ -26,6 +26,7 @@ import ghidra.program.model.address.*; import ghidra.program.model.mem.*; import ghidra.trace.database.memory.DBTraceMemorySpace; import ghidra.trace.model.memory.TraceMemorySpaceInputStream; +import ghidra.util.LockHold; import ghidra.util.MathUtilities; public abstract class AbstractDBTraceProgramViewMemoryBlock implements MemoryBlock { @@ -96,6 +97,19 @@ public abstract class AbstractDBTraceProgramViewMemoryBlock implements MemoryBlo private final List info = Collections.singletonList(new MyMemoryBlockSourceInfo()); + private static final int CACHE_PAGE_COUNT = 3; + private final ByteCache cache = new ByteCache(CACHE_PAGE_COUNT) { + @Override + protected int doLoad(Address address, ByteBuffer buf) throws MemoryAccessException { + DBTraceMemorySpace space = + program.trace.getMemoryManager().getMemorySpace(getAddressSpace(), false); + if (space == null) { + throw new MemoryAccessException("Space does not exist"); + } + return space.getViewBytes(program.snap, address, buf); + } + }; + protected AbstractDBTraceProgramViewMemoryBlock(DBTraceProgramView program) { this.program = program; } @@ -106,6 +120,15 @@ public abstract class AbstractDBTraceProgramViewMemoryBlock implements MemoryBlo return getStart().getAddressSpace(); } + /** + * Should be called when the snap changes or when bytes change + */ + protected void invalidateBytesCache(AddressRange range) { + if (range == null || range.intersects(getAddressRange())) { + cache.invalidate(range); + } + } + protected DBTraceMemorySpace getMemorySpace() { return program.trace.getMemoryManager().getMemorySpace(getAddressSpace(), false); } @@ -167,20 +190,13 @@ public abstract class AbstractDBTraceProgramViewMemoryBlock implements MemoryBlo @Override public byte getByte(Address addr) throws MemoryAccessException { - AddressRange range = getAddressRange(); - if (!range.contains(addr)) { - throw new MemoryAccessException(); + try (LockHold hold = program.trace.lockRead()) { + AddressRange range = getAddressRange(); + if (!range.contains(addr)) { + throw new MemoryAccessException(); + } + return cache.read(addr); } - DBTraceMemorySpace space = - program.trace.getMemoryManager().getMemorySpace(range.getAddressSpace(), false); - if (space == null) { - throw new MemoryAccessException("Space does not exist"); - } - ByteBuffer buf = ByteBuffer.allocate(1); - if (space.getViewBytes(program.snap, addr, buf) != 1) { - throw new MemoryAccessException(); - } - return buf.get(0); } @Override @@ -190,17 +206,22 @@ public abstract class AbstractDBTraceProgramViewMemoryBlock implements MemoryBlo @Override public int getBytes(Address addr, byte[] b, int off, int len) throws MemoryAccessException { - AddressRange range = getAddressRange(); - if (!range.contains(addr)) { - throw new MemoryAccessException(); + try (LockHold hold = program.trace.lockRead()) { + AddressRange range = getAddressRange(); + if (!range.contains(addr)) { + throw new MemoryAccessException(); + } + if (cache.canCache(addr, len)) { + return cache.read(addr, ByteBuffer.wrap(b, off, len)); + } + DBTraceMemorySpace space = + program.trace.getMemoryManager().getMemorySpace(range.getAddressSpace(), false); + if (space == null) { + throw new MemoryAccessException("Space does not exist"); + } + len = MathUtilities.unsignedMin(len, range.getMaxAddress().subtract(addr) + 1); + return space.getViewBytes(program.snap, addr, ByteBuffer.wrap(b, off, len)); } - DBTraceMemorySpace space = - program.trace.getMemoryManager().getMemorySpace(range.getAddressSpace(), false); - if (space == null) { - throw new MemoryAccessException("Space does not exist"); - } - len = MathUtilities.unsignedMin(len, range.getMaxAddress().subtract(addr) + 1); - return space.getViewBytes(program.snap, addr, ByteBuffer.wrap(b, off, len)); } @Override @@ -217,6 +238,7 @@ public abstract class AbstractDBTraceProgramViewMemoryBlock implements MemoryBlo @Override public int putBytes(Address addr, byte[] b, int off, int len) throws MemoryAccessException { + // NB. The trace will notify us of the write, and we invalidate then AddressRange range = getAddressRange(); if (!range.contains(addr)) { throw new MemoryAccessException(); diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/ByteCache.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/ByteCache.java new file mode 100644 index 0000000000..1fbdfc2db0 --- /dev/null +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/ByteCache.java @@ -0,0 +1,152 @@ +/* ### + * IP: GHIDRA + * + * 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. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package ghidra.trace.database.program; + +import java.nio.ByteBuffer; + +import ghidra.program.model.address.Address; +import ghidra.program.model.address.AddressRange; +import ghidra.program.model.mem.MemoryAccessException; +import ghidra.util.MathUtilities; + +public abstract class ByteCache { + public static final int BITS = 12; + public static final long OFFSET_MASK = -1L << BITS; + public static final int SIZE = 1 << BITS; + + private class Page { + private volatile boolean valid = false; + private Address start = null; + private final byte[] bytes = new byte[SIZE]; + private final ByteBuffer buf = ByteBuffer.wrap(bytes); + private int len; + + public final boolean contains(Address address, int length) { + if (!valid || start == null) { + return false; + } + long offset = address.subtract(start); + return Long.compareUnsigned(offset + length, len) < 0; + } + + public final int load(Address address, int length) throws MemoryAccessException { + valid = false; + start = address.getNewAddress(address.getOffset() & OFFSET_MASK); + long offset = address.subtract(start); + buf.clear(); // NB. Do not limit. Cache what we can. + len = doLoad(address, buf); + if (len < offset + length) { + throw new MemoryAccessException(); + } + valid = true; + return (int) offset; + } + + public void invalidate(AddressRange range) { + // TODO: Is it worth it to check for intersection? + valid = false; + } + } + + private final int pageCount; + private final Page[] pages; + + public ByteCache(int pageCount) { + this.pageCount = pageCount; + pages = new Page[pageCount]; + for (int i = 0; i < pageCount; i++) { + pages[i] = newPage(); + } + } + + protected Page newPage() { + return new Page(); + } + + public boolean canCache(Address address, int len) { + long cacheBufOff = address.getOffset() & ~OFFSET_MASK; + return cacheBufOff + len < pageCount * SIZE; + } + + public byte read(Address address) throws MemoryAccessException { + Address pageStart = address.getNewAddress(address.getOffset() & OFFSET_MASK); + Page page = ensurePageCached(pageStart, 1); + int cacheBufOff = (int) address.subtract(pageStart); + return page.bytes[cacheBufOff]; + } + + public int read(Address address, ByteBuffer buf) throws MemoryAccessException { + long startOff = address.getOffset(); + long startPage = startOff & OFFSET_MASK; + int bufStart = buf.position(); + + long memOffset = startPage; + int cacheBufOff = (int) (startOff - startPage); + while (buf.hasRemaining()) { + int required = MathUtilities.unsignedMin(SIZE - cacheBufOff, buf.remaining()); + Page page = ensurePageCached(address.getNewAddress(memOffset), required); + buf.put(page.bytes, cacheBufOff, required); + memOffset += SIZE; + cacheBufOff = 0; + } + return buf.position() - bufStart; + } + + protected int choosePage(Address address, int len) { + for (int i = 0; i < pageCount; i++) { + Page page = pages[i]; + if (page.contains(address, len)) { + return i; + } + } + return -1; + } + + /** + * Ensure a page is cached + * + * @param address the start address of the page + * @param len the minimum number of bytes required from the page + * @return the chosen cache page + * @throws MemoryAccessException if the required bytes cannot be read + */ + private Page ensurePageCached(Address address, int len) throws MemoryAccessException { + int chosen = choosePage(address, len); + if (chosen == -1) { + pages[pageCount - 1].load(address, len); + chosen = pageCount - 1; + } + if (chosen == 0) { + return pages[0]; + } + synchronized (pages) { + Page temp = pages[chosen]; + pages[chosen] = pages[0]; + pages[0] = temp; + return temp; + } + } + + protected abstract int doLoad(Address address, ByteBuffer buf) throws MemoryAccessException; + + public void invalidate(AddressRange range) { + synchronized (pages) { + for (Page p : pages) { + p.invalidate(range); + } + } + } +} diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramView.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramView.java index 3986521988..30be8c21d1 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramView.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramView.java @@ -1637,6 +1637,10 @@ public class DBTraceProgramView implements TraceProgramView { memory.updateRefreshBlocks(); } + public void updateBytesChanged(AddressRange range) { + memory.updateBytesChanged(range); + } + protected DomainObjectEventQueues getEventQueues(TraceAddressSpace space) { // TODO: Should there be views on other frames? // IIRC, this was an abandoned experiment for "register listings" diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramViewMemory.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramViewMemory.java index 46cfaa6e0e..582bce05b0 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramViewMemory.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramViewMemory.java @@ -67,6 +67,12 @@ public class DBTraceProgramViewMemory extends AbstractDBTraceProgramViewMemory { } } + @Override + void setSnap(long snap) { + super.setSnap(snap); + updateBytesChanged(null); + } + @Override protected void recomputeAddressSet() { AddressSet temp = new AddressSet(); @@ -155,4 +161,15 @@ public class DBTraceProgramViewMemory extends AbstractDBTraceProgramViewMemory { spaceBlocks.clear(); recomputeAddressSet(); } + + public void updateBytesChanged(AddressRange range) { + if (regionBlocks == null) { // order + return; + } + for (AbstractDBTraceProgramViewMemoryBlock block : forceFullView + ? spaceBlocks.values() + : regionBlocks.values()) { + block.invalidateBytesCache(range); + } + } }