Merge remote-tracking branch 'origin/GP-830-dragonmacher-symbol-table-add-remove--SQUASHED'

2026-05-10 05:39:36 +08:00 · 2021-04-07 09:50:13 -04:00
parent da94eb86bd b1d55498ec
commit cac2f00d41
7 changed files with 233 additions and 26 deletions
@@ -0,0 +1,85 @@
+/* ###
+ * 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.app.plugin.core.symtable;
+
+import java.util.*;
+
+import docking.widgets.table.AddRemoveListItem;
+import docking.widgets.table.threaded.TableAddRemoveStrategy;
+import docking.widgets.table.threaded.TableData;
+import ghidra.util.exception.CancelledException;
+import ghidra.util.task.TaskMonitor;
+
+/**
+ * This strategy attempts to optimize removal of db objects that have been deleted.  The issue with
+ * deleted db objects is that they may no longer have their attributes, which means we cannot
+ * use any of those attributes that may have been used as the basis for sorting.  We use the 
+ * table's sort to perform a binary search of existing symbols for removal.  If the binary search
+ * does not work, then removal operations will require slow list traversal.   Additionally, 
+ * some clients use proxy objects in add/remove list to signal which object needs to be removed, 
+ * since the original object is no longer available to the client.    Using these proxy objects
+ * in a binary search may lead to exceptions if the proxy has unsupported methods called when 
+ * searching.
+ * 
+ * <P>This strategy will has guilty knowledge of client proxy object usage.   The proxy objects
+ * are coded such that the {@code hashCode()} and {@code equals()} methods will match those 
+ * methods of the data's real objects.   
+ *
+ * @param <T> the row type
+ */
+public class SymbolTableAddRemoveStrategy<T> implements TableAddRemoveStrategy<T> {
+
+	@Override
+	public void process(List<AddRemoveListItem<T>> addRemoveList, TableData<T> tableData,
+			TaskMonitor monitor) throws CancelledException {
+
+		// 
+		// Hash map the existing values so that we can use any object inside the add/remove list
+		// as a key into this map to get the matching existing value.
+		//
+		Map<T, T> hashed = new HashMap<>();
+		for (T t : tableData) {
+			hashed.put(t, t);
+		}
+
+		int n = addRemoveList.size();
+		monitor.setMessage("Adding/Removing " + n + " items...");
+		monitor.initialize(n);
+		for (int i = 0; i < n; i++) {
+			AddRemoveListItem<T> item = addRemoveList.get(i);
+			T value = item.getValue();
+			if (item.isChange()) {
+				T toRemove = hashed.get(value);
+				if (toRemove != null) {
+					tableData.remove(toRemove);
+				}
+				tableData.insert(value);
+			}
+			else if (item.isRemove()) {
+				T toRemove = hashed.get(value);
+				if (toRemove != null) {
+					tableData.remove(toRemove);
+				}
+			}
+			else if (item.isAdd()) {
+				tableData.insert(value);
+			}
+			monitor.checkCanceled();
+			monitor.setProgress(i);
+		}
+		monitor.setMessage("Done adding/removing");
+	}
+}
@@ -18,6 +18,7 @@ package ghidra.app.plugin.core.symtable;
 import java.util.*;

 import docking.widgets.table.*;
+import docking.widgets.table.threaded.TableAddRemoveStrategy;
 import ghidra.app.cmd.function.DeleteFunctionCmd;
 import ghidra.app.cmd.label.DeleteLabelCmd;
 import ghidra.app.cmd.label.RenameLabelCmd;
@@ -60,6 +61,8 @@ class SymbolTableModel extends AddressBasedTableModel<Symbol> {
 	private ReferenceManager refMgr;
 	private Symbol lastSymbol;
 	private SymbolFilter filter;
+	private TableAddRemoveStrategy<Symbol> deletedDbObjectAddRemoveStrategy =
+		new SymbolTableAddRemoveStrategy<>();

 	SymbolTableModel(SymbolProvider provider, PluginTool tool) {
 		super("Symbols", tool, null, null);
@@ -88,6 +91,11 @@ class SymbolTableModel extends AddressBasedTableModel<Symbol> {
 		return descriptor;
 	}

+	@Override
+	protected TableAddRemoveStrategy<Symbol> getAddRemoveStrategy() {
+		return deletedDbObjectAddRemoveStrategy;
+	}
+
 	void setFilter(SymbolFilter filter) {
 		this.filter = filter;
 		reload();
@@ -0,0 +1,61 @@
+/* ###
+ * 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 docking.widgets.table.threaded;
+
+import java.util.List;
+
+import docking.widgets.table.AddRemoveListItem;
+import ghidra.util.exception.CancelledException;
+import ghidra.util.task.TaskMonitor;
+
+/**
+ * A strategy that uses the table's sort state to perform a binary search of items to be added
+ * and removed.   
+ *
+ * @param <T> the row type
+ */
+public class DefaultAddRemoveStrategy<T> implements TableAddRemoveStrategy<T> {
+
+	@Override
+	public void process(List<AddRemoveListItem<T>> addRemoveList, TableData<T> updatedData,
+			TaskMonitor monitor) throws CancelledException {
+
+		int n = addRemoveList.size();
+		monitor.setMessage("Adding/Removing " + n + " items...");
+		monitor.initialize(n);
+
+		// Note: this class does not directly perform a binary such, but instead relies on that
+		//       work to be done by the call to TableData.remove()
+		for (int i = 0; i < n; i++) {
+			AddRemoveListItem<T> item = addRemoveList.get(i);
+			T value = item.getValue();
+			if (item.isChange()) {
+				updatedData.remove(value);
+				updatedData.insert(value);
+			}
+			else if (item.isRemove()) {
+				updatedData.remove(value);
+			}
+			else if (item.isAdd()) {
+				updatedData.insert(value);
+			}
+			monitor.checkCanceled();
+			monitor.setProgress(i);
+		}
+		monitor.setMessage("Done adding/removing");
+	}
+
+}
@@ -0,0 +1,40 @@
+/* ###
+ * 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 docking.widgets.table.threaded;
+
+import java.util.List;
+
+import docking.widgets.table.AddRemoveListItem;
+import ghidra.util.exception.CancelledException;
+import ghidra.util.task.TaskMonitor;
+
+/**
+ * A strategy to perform table add and remove updates
+ *
+ * @param <T> the row type
+ */
+public interface TableAddRemoveStrategy<T> {
+
+	/**
+	 * Adds to and removes from the table data those items in the given add/remove list
+	 * @param addRemoveList the items to add/remove
+	 * @param tableData the table's data
+	 * @param monitor the monitor
+	 * @throws CancelledException if the monitor is cancelled
+	 */
+	public void process(List<AddRemoveListItem<T>> addRemoveList, TableData<T> tableData,
+			TaskMonitor monitor) throws CancelledException;
+}
@@ -137,7 +137,7 @@ public class TableData<ROW_OBJECT> implements Iterable<ROW_OBJECT> {
 	 * @param t the item
 	 * @return the index
 	 */
-	int indexOf(ROW_OBJECT t) {
+	public int indexOf(ROW_OBJECT t) {
 		if (!sortContext.isUnsorted()) {
 			Comparator<ROW_OBJECT> comparator = sortContext.getComparator();
 			return Collections.binarySearch(data, t, comparator);
@@ -153,7 +153,7 @@ public class TableData<ROW_OBJECT> implements Iterable<ROW_OBJECT> {
 		return -1;
 	}

-	boolean remove(ROW_OBJECT t) {
+	public boolean remove(ROW_OBJECT t) {
 		if (source != null) {
 			source.remove(t);
 		}
@@ -186,7 +186,7 @@ public class TableData<ROW_OBJECT> implements Iterable<ROW_OBJECT> {
 	 * 
 	 * @param value the row Object to insert
 	 */
-	void insert(ROW_OBJECT value) {
+	public void insert(ROW_OBJECT value) {

 		if (source != null) {
 			// always update the master data
@@ -553,31 +553,14 @@ public class TableUpdateJob<T> {
 	 */
 	private void doProcessAddRemoves() throws CancelledException {

-		int n = addRemoveList.size();
-		monitor.setMessage("Adding/Removing " + n + " items...");
-		monitor.initialize(n);
-
 		initializeSortCache();
-
-		for (int i = 0; i < n; i++) {
-			AddRemoveListItem<T> item = addRemoveList.get(i);
-			T value = item.getValue();
-			if (item.isChange()) {
-				updatedData.remove(value);
-				updatedData.insert(value);
-			}
-			else if (item.isRemove()) {
-				updatedData.remove(value);
-			}
-			else if (item.isAdd()) {
-				updatedData.insert(value);
-			}
-			monitor.checkCanceled();
-			monitor.setProgress(i);
+		try {
+			TableAddRemoveStrategy<T> strategy = model.getAddRemoveStrategy();
+			strategy.process(addRemoveList, updatedData, monitor);
+		}
+		finally {
+			clearSortCache();
 		}
-		monitor.setMessage("Done adding/removing");
-
-		clearSortCache();
 	}

 	/** When sorting we cache column value lookups to increase speed. */
@@ -90,6 +90,8 @@ public abstract class ThreadedTableModel<ROW_OBJECT, DATA_SOURCE>
 	private volatile Worker worker; // only created as needed (if we are incremental)
 	private int minUpdateDelayMillis;
 	private int maxUpdateDelayMillis;
+	private TableAddRemoveStrategy<ROW_OBJECT> binarySearchAddRemoveStrategy =
+		new DefaultAddRemoveStrategy<>();

 	protected ThreadedTableModel(String modelName, ServiceProvider serviceProvider) {
 		this(modelName, serviceProvider, null);
@@ -510,6 +512,17 @@ public abstract class ThreadedTableModel<ROW_OBJECT, DATA_SOURCE>

 	/**
 	 * Removes the specified object from this model and schedules an update.
+	 * 
+	 * <P>Note: for this method to function correctly, the given object must compare as 
+	 * {@link #equals(Object)} and have the same {@link #hashCode()} as the object to be removed 
+	 * from the table data.   This allows clients to create proxy objects to pass into this method,
+	 * as long as they honor those requirements.    
+	 * 
+	 * <P>If this model's data is sorted, then a binary search will be used to locate the item
+	 * to be removed.  However, for this to work, all field used to sort the data must still be 
+	 * available from the original object and must be the same values.   If this is not true, then
+	 * the binary search will not work and a brute force search will be used.
+	 * 
 	 * @param obj the object to remove
 	 */
 	public void removeObject(ROW_OBJECT obj) {
@@ -786,6 +799,23 @@ public abstract class ThreadedTableModel<ROW_OBJECT, DATA_SOURCE>
 		updateManager.setTaskMonitor(monitor);
 	}

+	/**
+	 * Returns the strategy to use for performing adds and removes to this table.   Subclasses can
+	 * override this method to customize this process for their particular type of data.   See
+	 * the implementations of {@link TableAddRemoveStrategy} for details.
+	 * 
+	 * <P>Note: The default add/remove strategy assumes that objects to be removed will be the 
+	 * same instance that is in the list of this model.   This allows the {@link #equals(Object)} 
+	 * and {@link #hashCode()} to be used when removing the object from the list.   If you model 
+	 * does not pass the same instance into {@link #removeObject(Object)}, then you will need to 
+	 * update your add/remove strategy accordingly.
+	 * 
+	 * @return the strategy
+	 */
+	protected TableAddRemoveStrategy<ROW_OBJECT> getAddRemoveStrategy() {
+		return binarySearchAddRemoveStrategy;
+	}
+
 	public void setIncrementalTaskMonitor(TaskMonitor monitor) {
 		SystemUtilities.assertTrue(loadIncrementally, "Cannot set an incremental task monitor " +
 			"on a table that was not constructed to load incrementally");