From c9c749b39ca775d6e6b0e17fa8e9ecf789620ab1 Mon Sep 17 00:00:00 2001 From: Dan <46821332+nsadeveloper789@users.noreply.github.com> Date: Fri, 23 Sep 2022 15:38:12 -0400 Subject: [PATCH] GP-2595: Make TraceTimeViewport receives updates directly rather than via change listener. --- .../core/debug/DebuggerCoordinates.java | 15 -- .../DefaultPcodeDebuggerMemoryAccess.java | 2 +- .../DefaultPcodeDebuggerRegistersAccess.java | 2 +- .../workflow/DisassembleAtPcDebuggerBot.java | 3 +- .../trace/data/AbstractPcodeTraceAccess.java | 5 +- .../data/AbstractPcodeTraceDataAccess.java | 2 +- .../data/DefaultPcodeTraceMemoryAccess.java | 2 +- .../DefaultPcodeTraceRegistersAccess.java | 2 +- .../java/ghidra/trace/database/DBTrace.java | 79 ++++--- .../DBTraceTimeViewport.java} | 144 ++++-------- .../database/memory/DBTraceMemorySpace.java | 8 +- .../database/program/DBTraceProgramView.java | 12 +- .../program/DBTraceProgramViewRegisters.java | 2 +- .../trace/database/time/DBTraceSnapshot.java | 12 +- .../database/time/DBTraceTimeManager.java | 27 ++- .../main/java/ghidra/trace/model/Trace.java | 2 + .../{util => model}/TraceTimeViewport.java | 23 +- .../trace/model/program/TraceProgramView.java | 2 +- .../java/ghidra/trace/util/CopyOnWrite.java | 212 ++++++++++++++++++ .../DBTraceTimeViewportTest.java} | 15 +- .../ghidra/util/datastruct/ListenerMap.java | 4 + 21 files changed, 389 insertions(+), 186 deletions(-) rename Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/{util/DefaultTraceTimeViewport.java => database/DBTraceTimeViewport.java} (76%) rename Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/{util => model}/TraceTimeViewport.java (92%) create mode 100644 Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/CopyOnWrite.java rename Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/{util/DefaultTraceTimeViewportTest.java => database/DBTraceTimeViewportTest.java} (85%) diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/DebuggerCoordinates.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/DebuggerCoordinates.java index b1b5b2b46c..ac6e4a280d 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/DebuggerCoordinates.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/DebuggerCoordinates.java @@ -42,8 +42,6 @@ import ghidra.trace.model.thread.TraceObjectThread; import ghidra.trace.model.thread.TraceThread; import ghidra.trace.model.time.TraceSnapshot; import ghidra.trace.model.time.schedule.TraceSchedule; -import ghidra.trace.util.DefaultTraceTimeViewport; -import ghidra.trace.util.TraceTimeViewport; import ghidra.util.Msg; import ghidra.util.NotOwnerException; @@ -97,7 +95,6 @@ public class DebuggerCoordinates { private final int hash; private Long viewSnap; - private DefaultTraceTimeViewport viewport; private TraceObject registerContainer; DebuggerCoordinates(Trace trace, TracePlatform platform, TraceRecorder recorder, @@ -580,18 +577,6 @@ public class DebuggerCoordinates { return viewSnap = snapshots.iterator().next().getKey(); } - public synchronized TraceTimeViewport getViewport() { - if (viewport != null) { - return viewport; - } - if (trace == null) { - return null; - } - viewport = new DefaultTraceTimeViewport(trace); - viewport.setSnap(getViewSnap()); - return viewport; - } - public void writeDataState(PluginTool tool, SaveState saveState, String key) { if (this == NOWHERE) { return; diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/emulation/data/DefaultPcodeDebuggerMemoryAccess.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/emulation/data/DefaultPcodeDebuggerMemoryAccess.java index 8b1a944c19..874ac9f995 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/emulation/data/DefaultPcodeDebuggerMemoryAccess.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/emulation/data/DefaultPcodeDebuggerMemoryAccess.java @@ -32,8 +32,8 @@ import ghidra.program.model.listing.Program; import ghidra.program.model.mem.Memory; import ghidra.program.model.mem.MemoryAccessException; import ghidra.trace.model.Trace; +import ghidra.trace.model.TraceTimeViewport; import ghidra.trace.model.guest.TracePlatform; -import ghidra.trace.util.TraceTimeViewport; import ghidra.util.MathUtilities; import ghidra.util.Msg; import ghidra.util.task.TaskMonitor; diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/emulation/data/DefaultPcodeDebuggerRegistersAccess.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/emulation/data/DefaultPcodeDebuggerRegistersAccess.java index a2d6854ab1..345ad31b5b 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/emulation/data/DefaultPcodeDebuggerRegistersAccess.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/service/emulation/data/DefaultPcodeDebuggerRegistersAccess.java @@ -25,9 +25,9 @@ import ghidra.pcode.exec.trace.data.DefaultPcodeTraceRegistersAccess; import ghidra.program.model.address.*; import ghidra.program.model.lang.Language; import ghidra.program.model.lang.Register; +import ghidra.trace.model.TraceTimeViewport; import ghidra.trace.model.guest.TracePlatform; import ghidra.trace.model.thread.TraceThread; -import ghidra.trace.util.TraceTimeViewport; import ghidra.util.Msg; /** diff --git a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/workflow/DisassembleAtPcDebuggerBot.java b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/workflow/DisassembleAtPcDebuggerBot.java index 246c2b70b0..bf198ff4b8 100644 --- a/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/workflow/DisassembleAtPcDebuggerBot.java +++ b/Ghidra/Debug/Debugger/src/main/java/ghidra/app/plugin/core/debug/workflow/DisassembleAtPcDebuggerBot.java @@ -38,10 +38,9 @@ import ghidra.program.model.address.*; import ghidra.program.model.data.PointerDataType; import ghidra.program.model.lang.Register; import ghidra.program.model.util.CodeUnitInsertionException; -import ghidra.trace.model.Trace; +import ghidra.trace.model.*; import ghidra.trace.model.Trace.TraceMemoryBytesChangeType; import ghidra.trace.model.Trace.TraceStackChangeType; -import ghidra.trace.model.TraceAddressSnapRange; import ghidra.trace.model.listing.*; import ghidra.trace.model.memory.*; import ghidra.trace.model.program.TraceProgramView; diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/pcode/exec/trace/data/AbstractPcodeTraceAccess.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/pcode/exec/trace/data/AbstractPcodeTraceAccess.java index 6eb24bfe28..b26f783f9a 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/pcode/exec/trace/data/AbstractPcodeTraceAccess.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/pcode/exec/trace/data/AbstractPcodeTraceAccess.java @@ -23,10 +23,9 @@ import org.apache.commons.lang3.tuple.Pair; import ghidra.pcode.emu.PcodeThread; import ghidra.program.model.lang.Language; import ghidra.trace.model.Trace; +import ghidra.trace.model.TraceTimeViewport; import ghidra.trace.model.guest.TracePlatform; import ghidra.trace.model.thread.TraceThread; -import ghidra.trace.util.DefaultTraceTimeViewport; -import ghidra.trace.util.TraceTimeViewport; /** * An abstract implementation of {@link PcodeTraceAccess} @@ -58,7 +57,7 @@ public abstract class AbstractPcodeTraceAccess viewports = new WeakHashCowSet<>(); protected DBTraceVariableSnapProgramView programView; - protected Map programViews = new WeakHashMap<>(); - protected Map fixedProgramViews = new WeakValueHashMap<>(); + protected Set programViews = new WeakHashCowSet<>(); + protected Set programViewsView = Collections.unmodifiableSet(programViews); + protected Map fixedProgramViews = new WeakValueHashCowMap<>(); + // NOTE: Can't pre-construct unmodifiableMap(fixedProgramViews), because values()' id changes protected ListenerSet viewListeners = new ListenerSet<>(TraceProgramViewListener.class); @@ -578,14 +585,10 @@ public class DBTrace extends DBCachedDomainObjectAdapter implements Trace, Trace // NOTE: The new viewport will need to read from the time manager during init DBTraceProgramView view; try (LockHold hold = lockRead()) { - synchronized (fixedProgramViews) { - view = fixedProgramViews.get(snap); - if (view != null) { - return view; - } + view = fixedProgramViews.computeIfAbsent(snap, s -> { Msg.debug(this, "Creating fixed view at snap=" + snap); - view = new DBTraceProgramView(this, snap, baseCompilerSpec); - } + return new DBTraceProgramView(this, snap, baseCompilerSpec); + }); } viewListeners.fire.viewCreated(view); return view; @@ -597,10 +600,8 @@ public class DBTrace extends DBCachedDomainObjectAdapter implements Trace, Trace // NOTE: The new viewport will need to read from the time manager during init DBTraceVariableSnapProgramView view; try (LockHold hold = lockRead()) { - synchronized (programViews) { - view = new DBTraceVariableSnapProgramView(this, snap, baseCompilerSpec); - programViews.put(view, null); - } + view = new DBTraceVariableSnapProgramView(this, snap, baseCompilerSpec); + programViews.add(view); } viewListeners.fire.viewCreated(view); return view; @@ -611,6 +612,15 @@ public class DBTrace extends DBCachedDomainObjectAdapter implements Trace, Trace return programView; } + @Override + public synchronized DBTraceTimeViewport createTimeViewport() { + try (LockHold hold = lockRead()) { + DBTraceTimeViewport view = new DBTraceTimeViewport(this); + viewports.add(view); + return view; + } + } + @Override public LockHold lockRead() { return LockHold.lock(rwLock.readLock()); @@ -742,25 +752,26 @@ public class DBTrace extends DBCachedDomainObjectAdapter implements Trace, Trace @Override public Collection getAllProgramViews() { - Collection all = new ArrayList<>(); - synchronized (programViews) { - all.addAll(programViews.keySet()); + /** + * Cannot pre-construct fixedProgramViewsView, because the UnmodifiableMap will cache + * values() on the first call, and the CowMap will change that with every mutation. Thus, + * the view would not see changes to the underlying map. + */ + return new CompositeCollection<>(programViewsView, + Collections.unmodifiableCollection(fixedProgramViews.values())); + } + + protected void allViewports(Consumer action) { + for (DBTraceTimeViewport viewport : viewports) { + action.accept(viewport); } - synchronized (fixedProgramViews) { - all.addAll(fixedProgramViews.values()); - } - return all; } protected void allViews(Consumer action) { - Collection all = new ArrayList<>(); - synchronized (programViews) { - all.addAll(programViews.keySet()); + for (DBTraceProgramView view : programViews) { + action.accept(view); } - synchronized (fixedProgramViews) { - all.addAll(fixedProgramViews.values()); - } - for (DBTraceProgramView view : all) { + for (DBTraceProgramView view : fixedProgramViews.values()) { action.accept(view); } } @@ -812,4 +823,16 @@ public class DBTrace extends DBCachedDomainObjectAdapter implements Trace, Trace public void updateViewsRefreshBlocks() { allViews(v -> v.updateMemoryRefreshBlocks()); } + + public void updateViewportsSnapshotAdded(TraceSnapshot snapshot) { + allViewports(v -> v.updateSnapshotAdded(snapshot)); + } + + public void updateViewportsSnapshotChanged(TraceSnapshot snapshot) { + allViewports(v -> v.updateSnapshotChanged(snapshot)); + } + + public void updateViewportsSnapshotDeleted(TraceSnapshot snapshot) { + allViewports(v -> v.updateSnapshotDeleted(snapshot)); + } } diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/DefaultTraceTimeViewport.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/DBTraceTimeViewport.java similarity index 76% rename from Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/DefaultTraceTimeViewport.java rename to Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/DBTraceTimeViewport.java index d59c0baa52..9a06a946a7 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/DefaultTraceTimeViewport.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/DBTraceTimeViewport.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package ghidra.trace.util; +package ghidra.trace.database; import java.util.*; import java.util.function.Function; @@ -21,19 +21,15 @@ import java.util.stream.Collectors; import com.google.common.collect.*; -import ghidra.framework.model.DomainObjectClosedListener; -import ghidra.framework.model.DomainObjectException; import ghidra.program.model.address.*; import ghidra.trace.model.Trace; -import ghidra.trace.model.Trace.TraceSnapshotChangeType; -import ghidra.trace.model.TraceDomainObjectListener; +import ghidra.trace.model.TraceTimeViewport; import ghidra.trace.model.program.TraceProgramView; import ghidra.trace.model.time.TraceSnapshot; import ghidra.trace.model.time.TraceTimeManager; import ghidra.trace.model.time.schedule.TraceSchedule; import ghidra.util.*; import ghidra.util.datastruct.ListenerSet; -import ghidra.util.exception.ClosedException; /** * Computes and tracks the "viewport" resulting from forking patterns encoded in snapshot schedules @@ -50,55 +46,7 @@ import ghidra.util.exception.ClosedException; * viewport. If complex, deep forking structures prove to be desirable, then this is an area for * optimization. */ -public class DefaultTraceTimeViewport implements TraceTimeViewport { - protected class ForSnapshotsListener extends TraceDomainObjectListener - implements DomainObjectClosedListener { - { - listenFor(TraceSnapshotChangeType.ADDED, ignoringClosed(this::snapshotAdded)); - listenFor(TraceSnapshotChangeType.CHANGED, ignoringClosed(this::snapshotChanged)); - listenFor(TraceSnapshotChangeType.DELETED, ignoringClosed(this::snapshotDeleted)); - } - - private AffectedObjectOnlyHandler ignoringClosed( - AffectedObjectOnlyHandler handler) { - return snapshot -> { - try { - handler.handle(snapshot); - } - catch (DomainObjectException e) { - if (e.getCause() instanceof ClosedException) { - Msg.warn(this, "Ignoring ClosedException in trace viewport update"); - } - else { - throw e; - } - } - }; - } - - private void snapshotAdded(TraceSnapshot snapshot) { - if (checkSnapshotAddedNeedsRefresh(snapshot)) { - refreshSnapRanges(); - } - } - - private void snapshotChanged(TraceSnapshot snapshot) { - if (checkSnapshotChangedNeedsRefresh(snapshot)) { - refreshSnapRanges(); - } - } - - private void snapshotDeleted(TraceSnapshot snapshot) { - if (checkSnapshotDeletedNeedsRefresh(snapshot)) { - refreshSnapRanges(); - } - } - - @Override - public void domainObjectClosed() { - trace.removeListener(this); - } - } +public class DBTraceTimeViewport implements TraceTimeViewport { protected final Trace trace; /** @@ -108,19 +56,16 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport { */ protected final List> ordered = new ArrayList<>(); protected final RangeSet spanSet = TreeRangeSet.create(); - protected final ForSnapshotsListener listener = new ForSnapshotsListener(); protected final ListenerSet changeListeners = new ListenerSet<>(Runnable.class); protected long snap = 0; - public DefaultTraceTimeViewport(Trace trace) { + protected DBTraceTimeViewport(Trace trace) { Range zero = Range.singleton(0L); spanSet.add(zero); ordered.add(zero); this.trace = trace; - trace.addCloseListener(listener); - trace.addListener(listener); } @Override @@ -194,7 +139,7 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport { } protected boolean isLower(long lower) { - try (LockHold hold = trace.lockRead()) { // May not be necessary + try (LockHold hold = trace.lockRead()) { synchronized (ordered) { Range range = spanSet.rangeContaining(lower); if (range == null) { @@ -283,6 +228,7 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport { changeListeners.fire.run(); } + @Override public void setSnap(long snap) { if (this.snap == snap) { return; @@ -291,48 +237,54 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport { refreshSnapRanges(); } - protected boolean checkSnapshotAddedNeedsRefresh(TraceSnapshot snapshot) { - try (LockHold hold = trace.lockRead()) { - synchronized (ordered) { - if (snapshot.getSchedule() == null) { - return false; - } - if (spanSet.contains(snapshot.getKey())) { - return true; - } - return false; - } + protected void updateSnapshotAdded(TraceSnapshot snapshot) { + if (checkSnapshotAddedNeedsRefresh(snapshot)) { + refreshSnapRanges(); } } + protected void updateSnapshotChanged(TraceSnapshot snapshot) { + if (checkSnapshotChangedNeedsRefresh(snapshot)) { + refreshSnapRanges(); + } + } + + protected void updateSnapshotDeleted(TraceSnapshot snapshot) { + if (checkSnapshotDeletedNeedsRefresh(snapshot)) { + refreshSnapRanges(); + } + } + + protected boolean checkSnapshotAddedNeedsRefresh(TraceSnapshot snapshot) { + if (snapshot.getSchedule() == null) { + return false; + } + if (spanSet.contains(snapshot.getKey())) { + return true; + } + return false; + } + protected boolean checkSnapshotChangedNeedsRefresh(TraceSnapshot snapshot) { - try (LockHold hold = trace.lockRead()) { - synchronized (ordered) { - if (isLower(snapshot.getKey())) { - return true; - } - if (spanSet.contains(snapshot.getKey()) && snapshot.getSchedule() != null) { - return true; - } - return false; - } + if (isLower(snapshot.getKey())) { + return true; } + if (spanSet.contains(snapshot.getKey()) && snapshot.getSchedule() != null) { + return true; + } + return false; } protected boolean checkSnapshotDeletedNeedsRefresh(TraceSnapshot snapshot) { - try (LockHold hold = trace.lockRead()) { - synchronized (ordered) { - if (isLower(snapshot.getKey())) { - return true; - } - return false; - } + if (isLower(snapshot.getKey())) { + return true; } + return false; } @Override public boolean isForked() { - try (LockHold hold = trace.lockRead()) { // May not be necessary + try (LockHold hold = trace.lockRead()) { synchronized (ordered) { return ordered.size() > 1; } @@ -340,7 +292,7 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport { } public List> getOrderedSpans() { - try (LockHold hold = trace.lockRead()) { // May not be necessary + try (LockHold hold = trace.lockRead()) { synchronized (ordered) { return List.copyOf(ordered); } @@ -348,17 +300,15 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport { } public List> getOrderedSpans(long snap) { - try (LockHold hold = trace.lockRead()) { // setSnap requires this - synchronized (ordered) { - setSnap(snap); - return getOrderedSpans(); - } + try (LockHold hold = trace.lockRead()) { + setSnap(snap); + return getOrderedSpans(); } } @Override public List getOrderedSnaps() { - try (LockHold hold = trace.lockRead()) { // May not be necessary + try (LockHold hold = trace.lockRead()) { synchronized (ordered) { return ordered .stream() @@ -370,7 +320,7 @@ public class DefaultTraceTimeViewport implements TraceTimeViewport { @Override public List getReversedSnaps() { - try (LockHold hold = trace.lockRead()) { // May not be necessary + try (LockHold hold = trace.lockRead()) { synchronized (ordered) { return Lists.reverse(ordered) .stream() 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..4b76d35545 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 @@ -30,8 +30,7 @@ import com.google.common.collect.Range; import db.DBHandle; import ghidra.program.model.address.*; import ghidra.program.model.mem.MemBuffer; -import ghidra.trace.database.DBTrace; -import ghidra.trace.database.DBTraceUtils; +import ghidra.trace.database.*; import ghidra.trace.database.DBTraceUtils.AddressRangeMapSetter; import ghidra.trace.database.DBTraceUtils.OffsetSnap; import ghidra.trace.database.listing.DBTraceCodeSpace; @@ -43,7 +42,6 @@ import ghidra.trace.model.*; import ghidra.trace.model.Trace.*; import ghidra.trace.model.memory.*; import ghidra.trace.model.thread.TraceThread; -import ghidra.trace.util.DefaultTraceTimeViewport; import ghidra.trace.util.TraceChangeRecord; import ghidra.util.*; import ghidra.util.AddressIteratorAdapter; @@ -95,7 +93,7 @@ public class DBTraceMemorySpace .build() .asMap(); - protected final DefaultTraceTimeViewport viewport; + protected final DBTraceTimeViewport viewport; public DBTraceMemorySpace(DBTraceMemoryManager manager, DBHandle dbh, AddressSpace space, DBTraceSpaceEntry ent, TraceThread thread) throws IOException, VersionException { @@ -135,7 +133,7 @@ public class DBTraceMemorySpace this.blocksByOffset = blockStore.getIndex(OffsetSnap.class, DBTraceMemoryBlockEntry.LOCATION_COLUMN); - this.viewport = new DefaultTraceTimeViewport(trace); + this.viewport = trace.createTimeViewport(); } @Override 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..7f250d712a 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 @@ -45,13 +45,14 @@ import ghidra.program.model.util.PropertyMapManager; import ghidra.program.util.ChangeManager; import ghidra.program.util.ProgramChangeRecord; import ghidra.trace.database.DBTrace; +import ghidra.trace.database.DBTraceTimeViewport; import ghidra.trace.database.listing.DBTraceCodeSpace; import ghidra.trace.database.listing.DBTraceDefinedUnitsView; import ghidra.trace.database.memory.DBTraceMemorySpace; import ghidra.trace.database.symbol.DBTraceFunctionSymbolView; +import ghidra.trace.model.*; import ghidra.trace.model.Trace.*; -import ghidra.trace.model.TraceAddressSnapRange; -import ghidra.trace.model.TraceDomainObjectListener; +import ghidra.trace.model.TraceTimeViewport.*; import ghidra.trace.model.bookmark.TraceBookmark; import ghidra.trace.model.bookmark.TraceBookmarkType; import ghidra.trace.model.data.TraceBasedDataTypeManager; @@ -61,8 +62,7 @@ import ghidra.trace.model.memory.TraceMemoryState; import ghidra.trace.model.program.TraceProgramView; import ghidra.trace.model.symbol.*; import ghidra.trace.model.thread.TraceThread; -import ghidra.trace.util.*; -import ghidra.trace.util.TraceTimeViewport.*; +import ghidra.trace.util.TraceAddressSpace; import ghidra.util.*; import ghidra.util.datastruct.WeakValueHashMap; import ghidra.util.exception.CancelledException; @@ -891,7 +891,7 @@ public class DBTraceProgramView implements TraceProgramView { protected final Map regViewsByThread; protected long snap; - protected final DefaultTraceTimeViewport viewport; + protected final DBTraceTimeViewport viewport; protected final Runnable viewportChangeListener = this::viewportChanged; // This is a strange thing @@ -910,7 +910,7 @@ public class DBTraceProgramView implements TraceProgramView { this.language = compilerSpec.getLanguage(); this.compilerSpec = compilerSpec; - this.viewport = new DefaultTraceTimeViewport(trace); + this.viewport = trace.createTimeViewport(); this.viewport.setSnap(snap); this.eventQueues = diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramViewRegisters.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramViewRegisters.java index 1b4a0623ba..11811fd2e8 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramViewRegisters.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/program/DBTraceProgramViewRegisters.java @@ -37,11 +37,11 @@ import ghidra.program.model.util.PropertyMapManager; import ghidra.trace.database.listing.DBTraceCodeSpace; import ghidra.trace.database.memory.DBTraceMemorySpace; import ghidra.trace.model.Trace; +import ghidra.trace.model.TraceTimeViewport; import ghidra.trace.model.data.TraceBasedDataTypeManager; import ghidra.trace.model.program.TraceProgramView; import ghidra.trace.model.program.TraceProgramViewMemory; import ghidra.trace.model.thread.TraceThread; -import ghidra.trace.util.TraceTimeViewport; import ghidra.util.exception.CancelledException; import ghidra.util.exception.DuplicateNameException; import ghidra.util.task.TaskMonitor; diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/time/DBTraceSnapshot.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/time/DBTraceSnapshot.java index c2a0274950..e2d1fb3c00 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/time/DBTraceSnapshot.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/time/DBTraceSnapshot.java @@ -115,9 +115,8 @@ public class DBTraceSnapshot extends DBAnnotatedObject implements TraceSnapshot try (LockHold hold = LockHold.lock(manager.lock.writeLock())) { this.realTime = millis; update(REAL_TIME_COLUMN); + manager.notifySnapshotChanged(this); } - manager.trace.setChanged( - new TraceChangeRecord<>(TraceSnapshotChangeType.CHANGED, null, this)); } @Override @@ -130,9 +129,8 @@ public class DBTraceSnapshot extends DBAnnotatedObject implements TraceSnapshot try (LockHold hold = LockHold.lock(manager.lock.writeLock())) { this.description = description; update(DESCRIPTION_COLUMN); + manager.notifySnapshotChanged(this); } - manager.trace.setChanged( - new TraceChangeRecord<>(TraceSnapshotChangeType.CHANGED, null, this)); } @Override @@ -152,9 +150,8 @@ public class DBTraceSnapshot extends DBAnnotatedObject implements TraceSnapshot threadKey = thread.getKey(); } update(THREAD_COLUMN); + manager.notifySnapshotChanged(this); } - manager.trace.setChanged( - new TraceChangeRecord<>(TraceSnapshotChangeType.CHANGED, null, this)); } @Override @@ -173,9 +170,8 @@ public class DBTraceSnapshot extends DBAnnotatedObject implements TraceSnapshot this.schedule = schedule; this.scheduleStr = schedule == null ? "" : schedule.toString(); update(SCHEDULE_COLUMN); + manager.notifySnapshotChanged(this); } - manager.trace.setChanged( - new TraceChangeRecord<>(TraceSnapshotChangeType.CHANGED, null, this)); } @Override diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/time/DBTraceTimeManager.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/time/DBTraceTimeManager.java index 9196d5f36b..9853d0f641 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/time/DBTraceTimeManager.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/database/time/DBTraceTimeManager.java @@ -68,6 +68,21 @@ public class DBTraceTimeManager implements TraceTimeManager, DBTraceManager { snapshotStore.invalidateCache(); } + protected void notifySnapshotAdded(DBTraceSnapshot snapshot) { + trace.updateViewportsSnapshotAdded(snapshot); + trace.setChanged(new TraceChangeRecord<>(TraceSnapshotChangeType.ADDED, null, snapshot)); + } + + protected void notifySnapshotChanged(DBTraceSnapshot snapshot) { + trace.updateViewportsSnapshotChanged(snapshot); + trace.setChanged(new TraceChangeRecord<>(TraceSnapshotChangeType.CHANGED, null, snapshot)); + } + + protected void notifySnapshotDeleted(DBTraceSnapshot snapshot) { + trace.updateViewportsSnapshotDeleted(snapshot); + trace.setChanged(new TraceChangeRecord<>(TraceSnapshotChangeType.DELETED, null, snapshot)); + } + @Override public DBTraceSnapshot createSnapshot(String description) { try (LockHold hold = LockHold.lock(lock.writeLock())) { @@ -77,8 +92,7 @@ public class DBTraceTimeManager implements TraceTimeManager, DBTraceManager { // Convention for first snap snapshot.setSchedule(TraceSchedule.snap(0)); } - trace.setChanged( - new TraceChangeRecord<>(TraceSnapshotChangeType.ADDED, null, snapshot)); + notifySnapshotAdded(snapshot); return snapshot; } } @@ -99,8 +113,7 @@ public class DBTraceTimeManager implements TraceTimeManager, DBTraceManager { // Convention for first snap snapshot.setSchedule(TraceSchedule.snap(0)); } - trace.setChanged( - new TraceChangeRecord<>(TraceSnapshotChangeType.ADDED, null, snapshot)); + notifySnapshotAdded(snapshot); } return snapshot; } @@ -142,7 +155,9 @@ public class DBTraceTimeManager implements TraceTimeManager, DBTraceManager { } public void deleteSnapshot(DBTraceSnapshot snapshot) { - snapshotStore.delete(snapshot); - trace.setChanged(new TraceChangeRecord<>(TraceSnapshotChangeType.DELETED, null, snapshot)); + try (LockHold hold = LockHold.lock(lock.writeLock())) { + snapshotStore.delete(snapshot); + notifySnapshotDeleted(snapshot); + } } } diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/Trace.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/Trace.java index 6857463248..3ca8d1915a 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/Trace.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/Trace.java @@ -462,6 +462,8 @@ public interface Trace extends DataTypeManagerDomainObject { */ TraceVariableSnapProgramView getProgramView(); + TraceTimeViewport createTimeViewport(); + void addProgramViewListener(TraceProgramViewListener listener); void removeProgramViewListener(TraceProgramViewListener listener); diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/TraceTimeViewport.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/TraceTimeViewport.java similarity index 92% rename from Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/TraceTimeViewport.java rename to Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/TraceTimeViewport.java index 5121d99725..1f5cb1b385 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/TraceTimeViewport.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/TraceTimeViewport.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package ghidra.trace.util; +package ghidra.trace.model; import java.util.*; import java.util.function.Function; @@ -102,8 +102,29 @@ public interface TraceTimeViewport { AddressSetView set(T t); } + /** + * Set the snapshot for this viewport + * + * @param snap the snap + */ + void setSnap(long snap); + + /** + * Add a listener for when the forking structure of this viewport changes + * + *

+ * This can occur when the snap changes or when any snapshot involved changes + * + * @param l the listener + */ void addChangeListener(Runnable l); + /** + * Remove a listener for forking structure changes + * + * @see #addChangeListener(Runnable) + * @param l the listener + */ void removeChangeListener(Runnable l); /** diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/program/TraceProgramView.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/program/TraceProgramView.java index dfb2ee86f7..b93b524fa6 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/program/TraceProgramView.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/model/program/TraceProgramView.java @@ -17,8 +17,8 @@ package ghidra.trace.model.program; import ghidra.program.model.listing.Program; import ghidra.trace.model.Trace; +import ghidra.trace.model.TraceTimeViewport; import ghidra.trace.model.thread.TraceThread; -import ghidra.trace.util.TraceTimeViewport; /** * View of a trace at a particular time, as a program diff --git a/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/CopyOnWrite.java b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/CopyOnWrite.java new file mode 100644 index 0000000000..9d2936d269 --- /dev/null +++ b/Ghidra/Debug/Framework-TraceModeling/src/main/java/ghidra/trace/util/CopyOnWrite.java @@ -0,0 +1,212 @@ +/* ### + * 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.util; + +import java.util.*; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.Function; + +import ghidra.util.datastruct.AbstractWeakValueMap; + +public interface CopyOnWrite { + abstract class AbstractCowMap implements Map { + private final AtomicReference> map; + + public AbstractCowMap() { + map = new AtomicReference<>(copyMap(Map.of())); + } + + /** + * Extension point: Create a mutable copy of the given map + * + * @param map the map to copy + * @return the mutable copy + */ + protected abstract Map copyMap(Map map); + + @Override + public int size() { + return map.get().size(); + } + + @Override + public boolean isEmpty() { + return map.get().isEmpty(); + } + + @Override + public boolean containsKey(Object key) { + return map.get().containsKey(key); + } + + @Override + public boolean containsValue(Object value) { + return map.get().containsValue(value); + } + + @Override + public V get(Object key) { + return map.get().get(key); + } + + @Override + public V put(K key, V value) { + return map.getAndUpdate(m -> { + Map withPut = copyMap(m); + withPut.put(key, value); + return withPut; + }).get(key); + } + + @Override + public V remove(Object key) { + return map.getAndUpdate(m -> { + Map withRemove = copyMap(m); + withRemove.remove(key); + return withRemove; + }).get(key); + } + + @Override + public void putAll(Map from) { + map.getAndUpdate(m -> { + Map withPutAll = copyMap(m); + withPutAll.putAll(from); + return withPutAll; + }); + } + + @Override + public void clear() { + map.set(copyMap(Map.of())); + } + + @Override + public Set keySet() { + return Collections.unmodifiableSet(map.get().keySet()); + } + + @Override + public Collection values() { + return Collections.unmodifiableCollection(map.get().values()); + } + + @Override + public Set> entrySet() { + return Collections.unmodifiableSet(map.get().entrySet()); + } + + @Override + public V computeIfAbsent(K key, Function mappingFunction) { + return map.getAndUpdate(m -> { + if (m.containsKey(key)) { + return m; + } + Map withComputeIfAbsent = copyMap(m); + withComputeIfAbsent.put(key, mappingFunction.apply(key)); + return withComputeIfAbsent; + }).get(key); + } + } + + class HashCowMap extends AbstractCowMap { + @Override + protected Map copyMap(Map map) { + return new HashMap<>(map); + } + } + + abstract class WeakValueAbstractCowMap extends AbstractWeakValueMap { + private final AbstractCowMap> refMap = newCowMap(); + + protected abstract AbstractCowMap> newCowMap(); + + @Override + protected Map> getRefMap() { + return refMap; + } + } + + class WeakValueHashCowMap extends WeakValueAbstractCowMap { + @Override + protected AbstractCowMap> newCowMap() { + return new HashCowMap<>(); + } + } + + /** + * Assumes elements use system hash equality, i.e., {@link E#equals()} is ignored + * + * @param the type of element in the weak set + */ + abstract class WeakAbstractCowSet extends AbstractSet { + private final WeakValueAbstractCowMap map = newWeakValueCowMap(); + + protected abstract WeakValueAbstractCowMap newWeakValueCowMap(); + + @Override + public Iterator iterator() { + return map.values().iterator(); + } + + @Override + public int size() { + return map.size(); + } + + @Override + public boolean isEmpty() { + return map.isEmpty(); + } + + @Override + public boolean contains(Object o) { + return map.get(System.identityHashCode(o)) == o; + } + + @Override + public boolean add(E e) { + return map.put(System.identityHashCode(e), e) != e; + } + + @Override + public boolean remove(Object o) { + return map.remove(System.identityHashCode(o)) == o; + } + + @Override + public void clear() { + map.clear(); + } + + @Override + public Object[] toArray() { + return map.values().toArray(); + } + + @Override + public T[] toArray(T[] a) { + return map.values().toArray(a); + } + } + + class WeakHashCowSet extends WeakAbstractCowSet { + @Override + protected WeakValueAbstractCowMap newWeakValueCowMap() { + return new WeakValueHashCowMap<>(); + } + } +} diff --git a/Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/util/DefaultTraceTimeViewportTest.java b/Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/database/DBTraceTimeViewportTest.java similarity index 85% rename from Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/util/DefaultTraceTimeViewportTest.java rename to Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/database/DBTraceTimeViewportTest.java index 8604f5713a..8ff4d34571 100644 --- a/Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/util/DefaultTraceTimeViewportTest.java +++ b/Ghidra/Debug/Framework-TraceModeling/src/test/java/ghidra/trace/database/DBTraceTimeViewportTest.java @@ -13,7 +13,7 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package ghidra.trace.util; +package ghidra.trace.database; import static org.junit.Assert.assertEquals; @@ -24,12 +24,11 @@ import org.junit.Test; import com.google.common.collect.*; import ghidra.test.AbstractGhidraHeadlessIntegrationTest; -import ghidra.trace.database.ToyDBTraceBuilder; import ghidra.trace.database.time.DBTraceTimeManager; import ghidra.trace.model.time.schedule.TraceSchedule; import ghidra.util.database.UndoableTransaction; -public class DefaultTraceTimeViewportTest extends AbstractGhidraHeadlessIntegrationTest { +public class DBTraceTimeViewportTest extends AbstractGhidraHeadlessIntegrationTest { public static > RangeSet rangeSetOf(List> ranges) { RangeSet result = TreeRangeSet.create(); ranges.forEach(result::add); @@ -39,7 +38,7 @@ public class DefaultTraceTimeViewportTest extends AbstractGhidraHeadlessIntegrat @Test public void testEmptyTime() throws Exception { try (ToyDBTraceBuilder tb = new ToyDBTraceBuilder("test", "Toy:BE:64:default")) { - DefaultTraceTimeViewport viewport = new DefaultTraceTimeViewport(tb.trace); + DBTraceTimeViewport viewport = tb.trace.createTimeViewport(); viewport.setSnap(10); assertEquals(rangeSetOf(List.of(Range.closed(Long.MIN_VALUE, 10L))), viewport.spanSet); } @@ -52,7 +51,7 @@ public class DefaultTraceTimeViewportTest extends AbstractGhidraHeadlessIntegrat tb.trace.getTimeManager().getSnapshot(0, true).setSchedule(TraceSchedule.snap(0)); } - DefaultTraceTimeViewport viewport = new DefaultTraceTimeViewport(tb.trace); + DBTraceTimeViewport viewport = tb.trace.createTimeViewport(); viewport.setSnap(10); assertEquals(rangeSetOf(List.of(Range.closed(0L, 10L))), viewport.spanSet); } @@ -67,7 +66,7 @@ public class DefaultTraceTimeViewportTest extends AbstractGhidraHeadlessIntegrat tm.getSnapshot(5, true).setSchedule(TraceSchedule.parse("4:1")); } - DefaultTraceTimeViewport viewport = new DefaultTraceTimeViewport(tb.trace); + DBTraceTimeViewport viewport = tb.trace.createTimeViewport(); viewport.setSnap(10); assertEquals(rangeSetOf(List.of(Range.closed(0L, 10L))), viewport.spanSet); } @@ -82,7 +81,7 @@ public class DefaultTraceTimeViewportTest extends AbstractGhidraHeadlessIntegrat tm.getSnapshot(Long.MIN_VALUE, true).setSchedule(TraceSchedule.parse("10:4")); } - DefaultTraceTimeViewport viewport = new DefaultTraceTimeViewport(tb.trace); + DBTraceTimeViewport viewport = tb.trace.createTimeViewport(); viewport.setSnap(Long.MIN_VALUE); assertEquals( rangeSetOf(List.of(Range.singleton(Long.MIN_VALUE), Range.closed(0L, 10L))), @@ -98,7 +97,7 @@ public class DefaultTraceTimeViewportTest extends AbstractGhidraHeadlessIntegrat tm.getSnapshot(Long.MIN_VALUE, true).setSchedule(TraceSchedule.parse("10:4")); } - DefaultTraceTimeViewport viewport = new DefaultTraceTimeViewport(tb.trace); + DBTraceTimeViewport viewport = tb.trace.createTimeViewport(); viewport.setSnap(Long.MIN_VALUE); assertEquals(rangeSetOf(List.of(Range.singleton(Long.MIN_VALUE))), viewport.spanSet); } diff --git a/Ghidra/Debug/ProposedUtils/src/main/java/ghidra/util/datastruct/ListenerMap.java b/Ghidra/Debug/ProposedUtils/src/main/java/ghidra/util/datastruct/ListenerMap.java index a7b4aea432..a1bf059731 100644 --- a/Ghidra/Debug/ProposedUtils/src/main/java/ghidra/util/datastruct/ListenerMap.java +++ b/Ghidra/Debug/ProposedUtils/src/main/java/ghidra/util/datastruct/ListenerMap.java @@ -219,6 +219,10 @@ public class ListenerMap { } protected Map createMap() { + /** + * TODO: This is potentially flawed: The removal modifies the map in place. It does not + * adhere to "copy on write." It does have its own concurrency considerations, though. + */ CacheBuilder builder = CacheBuilder.newBuilder() .removalListener(this::notifyRemoved) .weakValues()