GP-830 - Updated the threaded table model to allow subclasses to change how data adds and removes are performed. This fixes and exception and increases performance for the Symbol Table

This commit is contained in:
dragonmacher 2021-04-06 09:27:23 -04:00
parent 472ad40077
commit b1d55498ec
7 changed files with 233 additions and 26 deletions

View File

@ -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");
}
}

View File

@ -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();

View File

@ -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");
}
}

View File

@ -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;
}

View File

@ -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

View File

@ -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. */

View File

@ -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");