GT-3226 - Symbol Table - Performance improvements - review fixes:

convert SymbolRowObject to Symbol

Ghidra/Features/Base/src/main/java/ghidra/app/cmd/label/RenameLabelCmd.java

@@ -120,9 +120,14 @@ public class RenameLabelCmd implements Command {
 			}
 			else {
 				s.setName(newName, source);
-				if ((newName.length() != 0 && !newName.equals(s.getName())) ||
-					(newName.length() == 0 && s.getSource() != SourceType.DEFAULT)) {
-					errorMsg = "Rename failed - default names may not be used";
+
+				if (newName.length() == 0 && s.getSource() != SourceType.DEFAULT) {
+					errorMsg = "Rename failed - cannot set non-default symbol name to \"\"";
+					return false;
+				}
+
+				if (!newName.equals(s.getName())) {
+					errorMsg = "Rename failed";
 					return false;
 				}
 			}

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symtable/ReferenceProvider.java

@@ -85,9 +85,9 @@ class ReferenceProvider extends ComponentProviderAdapter {
 		}
 	}
 
-	void symbolRemoved(long symbolID) {
+	void symbolRemoved(Symbol symbol) {
 		if (isVisible()) {
-			referenceKeyModel.symbolRemoved(symbolID);
+			referenceKeyModel.symbolRemoved(symbol);
 		}
 	}
 
@@ -157,5 +157,4 @@ class ReferenceProvider extends ComponentProviderAdapter {
 	public void updateTitle() {
 		setSubTitle(generateSubTitle());
 	}
-
 }

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symtable/SymbolEditor.java

@@ -39,9 +39,9 @@ class SymbolEditor extends DefaultCellEditor {
 	@Override
 	public Component getTableCellEditorComponent(JTable table, Object value, boolean isSelected,
 			int row, int column) {
-		if (value instanceof SymbolTableNameValue) {
-			SymbolTableNameValue cellValue = (SymbolTableNameValue) value;
-			Symbol symbol = cellValue.getSymbol();
+
+		Symbol symbol = (Symbol) value;
+		if (symbol != null) {
 			symbolField.setText(symbol.getName());
 		}
 		else {

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symtable/SymbolPanel.java

@@ -46,8 +46,8 @@ class SymbolPanel extends JPanel {
 	private GhidraTable symTable;
 	private TableModelListener listener;
 	private FilterDialog filterDialog;
-	private GhidraThreadedTablePanel<SymbolRowObject> threadedTablePanel;
-	private GhidraTableFilterPanel<SymbolRowObject> tableFilterPanel;
+	private GhidraThreadedTablePanel<Symbol> threadedTablePanel;
+	private GhidraTableFilterPanel<Symbol> tableFilterPanel;
 
 	SymbolPanel(SymbolProvider provider, SymbolTableModel model, SymbolRenderer renderer,
 			final PluginTool tool, GoToService gotoService) {
@@ -116,9 +116,9 @@ class SymbolPanel extends JPanel {
 		return tableFilterPanel;
 	}
 
-	protected RowFilterTransformer<SymbolRowObject> updateRowDataTransformer(boolean nameOnly) {
+	protected RowFilterTransformer<Symbol> updateRowDataTransformer(boolean nameOnly) {
 		if (nameOnly) {
-			return new NameOnlyRowTransformer(tableModel);
+			return new NameOnlyRowTransformer();
 		}
 
 		return new DefaultRowFilterTransformer<>(tableModel, symTable.getColumnModel());
@@ -185,7 +185,7 @@ class SymbolPanel extends JPanel {
 		return symTable.getRowCount();
 	}
 
-	List<SymbolRowObject> getSelectedSymbolKeys() {
+	List<Symbol> getSelectedSymbolKeys() {
 		int[] rows = symTable.getSelectedRows();
 		return tableModel.getRowObjects(rows);
 	}
@@ -198,21 +198,16 @@ class SymbolPanel extends JPanel {
 // Inner Classes
 //==================================================================================================
 
-	private static class NameOnlyRowTransformer implements RowFilterTransformer<SymbolRowObject> {
+	private static class NameOnlyRowTransformer implements RowFilterTransformer<Symbol> {
 		private List<String> list = new ArrayList<>();
-		private SymbolTableModel model;
-
-		NameOnlyRowTransformer(SymbolTableModel model) {
-			this.model = model;
-		}
 
 		@Override
-		public List<String> transform(SymbolRowObject rowObject) {
+		public List<String> transform(Symbol rowObject) {
 			list.clear();
-			Object value = model.getColumnValueForRow(rowObject, SymbolTableModel.LABEL_COL);
-			if (value != null) {
-				// the toString() returns the value for the symbol, which may be cached
-				list.add(value.toString());
+			if (rowObject != null) {
+				// The toString() returns the name for the symbol, which may be cached.  Calling
+				// toString() will also avoid locking for cached values.
+				list.add(rowObject.toString());
 			}
 			return list;
 		}

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symtable/SymbolProvider.java

@@ -74,17 +74,18 @@ class SymbolProvider extends ComponentProviderAdapter {
 		if (program == null) {
 			return null;
 		}
-		List<SymbolRowObject> rowObjects = symbolPanel.getSelectedSymbolKeys();
+
+		List<Symbol> rowObjects = symbolPanel.getSelectedSymbolKeys();
 		long[] symbolIDs = new long[rowObjects.size()];
 		int index = 0;
-		for (SymbolRowObject obj : rowObjects) {
-			symbolIDs[index++] = obj.getKey();
+		for (Symbol obj : rowObjects) {
+			symbolIDs[index++] = obj.getID();
 		}
 		return new ProgramSymbolActionContext(this, program, symbolIDs, getTable());
 	}
 
 	void deleteSymbols() {
-		List<SymbolRowObject> rowObjects = symbolPanel.getSelectedSymbolKeys();
+		List<Symbol> rowObjects = symbolPanel.getSelectedSymbolKeys();
 		symbolKeyModel.delete(rowObjects);
 	}
 
@@ -93,15 +94,15 @@ class SymbolProvider extends ComponentProviderAdapter {
 	}
 
 	Symbol getCurrentSymbol() {
-		List<SymbolRowObject> rowObjects = symbolPanel.getSelectedSymbolKeys();
+		List<Symbol> rowObjects = symbolPanel.getSelectedSymbolKeys();
 		if (rowObjects != null && rowObjects.size() >= 1) {
-			return symbolKeyModel.getSymbol(rowObjects.get(0).getKey());
+			return rowObjects.get(0);
 		}
 		return null;
 	}
 
 	Symbol getSymbolForRow(int row) {
-		return symbolKeyModel.getRowObject(row).getSymbol();
+		return symbolKeyModel.getRowObject(row);
 	}
 
 	void setCurrentSymbol(Symbol symbol) {
@@ -136,12 +137,6 @@ class SymbolProvider extends ComponentProviderAdapter {
 		}
 	}
 
-	void symbolRemoved(long symbolId) {
-		if (isVisible()) {
-			symbolKeyModel.symbolRemoved(symbolId);
-		}
-	}
-
 	void symbolChanged(Symbol s) {
 		if (isVisible()) {
 			symbolKeyModel.symbolChanged(s);

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symtable/SymbolReferenceModel.java

@@ -131,8 +131,8 @@ public class SymbolReferenceModel extends AddressBasedTableModel<Reference> {
 		checkRefs(symbol);
 	}
 
-	void symbolRemoved(long symbolID) {
-		if (currentSymbol != null && currentSymbol.getID() == symbolID) {
+	void symbolRemoved(Symbol symbol) {
+		if (currentSymbol != null && currentSymbol.getID() == symbol.getID()) {
 			setCurrentSymbol(null);
 		}
 	}

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symtable/SymbolRenderer.java

@@ -52,10 +52,6 @@ class SymbolRenderer extends GhidraTableCellRenderer {
 		if (value == null && column == SymbolTableModel.LABEL_COL) {
 			setText("<< REMOVED >>");
 		}
-		else if (value instanceof SymbolTableNameValue) {
-			Symbol symbol = ((SymbolTableNameValue) value).getSymbol();
-			handleSymbol(symbol, isSelected);
-		}
 		else if (value instanceof Symbol) {
 			handleSymbol(value, isSelected);
 		}

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symtable/SymbolRowObject.java

@@ -1,76 +0,0 @@
-/* ###
- * 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 ghidra.program.model.symbol.Symbol;
-
-class SymbolRowObject implements Comparable<SymbolRowObject> {
-
-	// symbol can be null after it is deleted
-	private final Symbol symbol;
-	private final long key;
-
-	SymbolRowObject(Symbol s) {
-		this.symbol = s;
-		this.key = s.getID();
-	}
-
-	// this constructor is used to create a row object to serve as a key for deleting items
-	// in the model after a symbol has been deleted
-	SymbolRowObject(long symbolId) {
-		this.symbol = null;
-		this.key = symbolId;
-	}
-
-	Symbol getSymbol() {
-		return symbol;
-	}
-
-	long getKey() {
-		return key;
-	}
-
-	@Override
-	public int hashCode() {
-		final int prime = 31;
-		int result = 1;
-		result = prime * result + (int) (key ^ (key >>> 32));
-		return result;
-	}
-
-	@Override
-	public boolean equals(Object obj) {
-		if (this == obj) {
-			return true;
-		}
-		if (obj == null) {
-			return false;
-		}
-		if (getClass() != obj.getClass()) {
-			return false;
-		}
-		SymbolRowObject other = (SymbolRowObject) obj;
-		if (key != other.key) {
-			return false;
-		}
-		return true;
-	}
-
-	@Override
-	public int compareTo(SymbolRowObject o) {
-		return ((Long) key).compareTo(o.key);
-	}
-}

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symtable/SymbolRowObjectToAddressTableRowMapper.java

@@ -1,6 +1,5 @@
 /* ###
  * IP: GHIDRA
- * REVIEWED: YES
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -20,21 +19,17 @@ import ghidra.framework.plugintool.ServiceProvider;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.listing.Program;
 import ghidra.program.model.symbol.Symbol;
-import ghidra.program.model.symbol.SymbolTable;
 import ghidra.util.table.ProgramLocationTableRowMapper;
 
-public class SymbolRowObjectToAddressTableRowMapper extends
-		ProgramLocationTableRowMapper<SymbolRowObject, Address> {
+public class SymbolRowObjectToAddressTableRowMapper
+		extends ProgramLocationTableRowMapper<Symbol, Address> {
 
 	@Override
-	public Address map(SymbolRowObject rowObject, Program data, ServiceProvider serviceProvider) {
-		SymbolTable symbolTable = data.getSymbolTable();
-		Symbol symbol = symbolTable.getSymbol(rowObject.getKey());
-		if (symbol == null) {
+	public Address map(Symbol rowObject, Program data, ServiceProvider serviceProvider) {
+		if (rowObject == null) {
 			return null;
 		}
-
-		return symbol.getAddress();
+		return rowObject.getAddress();
 	}
 
 }

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symtable/SymbolRowObjectToProgramLocationTableRowMapper.java

@@ -1,6 +1,5 @@
 /* ###
  * IP: GHIDRA
- * REVIEWED: YES
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -19,23 +18,15 @@ package ghidra.app.plugin.core.symtable;
 import ghidra.framework.plugintool.ServiceProvider;
 import ghidra.program.model.listing.Program;
 import ghidra.program.model.symbol.Symbol;
-import ghidra.program.model.symbol.SymbolTable;
 import ghidra.program.util.ProgramLocation;
 import ghidra.util.table.ProgramLocationTableRowMapper;
 
-public class SymbolRowObjectToProgramLocationTableRowMapper extends
-		ProgramLocationTableRowMapper<SymbolRowObject, ProgramLocation> {
+public class SymbolRowObjectToProgramLocationTableRowMapper
+		extends ProgramLocationTableRowMapper<Symbol, ProgramLocation> {
 
 	@Override
-	public ProgramLocation map(SymbolRowObject rowObject, Program data,
-			ServiceProvider serviceProvider) {
-		SymbolTable symbolTable = data.getSymbolTable();
-		Symbol symbol = symbolTable.getSymbol(rowObject.getKey());
-		if (symbol == null) {
-			return null;
-		}
-
-		return symbol.getProgramLocation();
+	public ProgramLocation map(Symbol rowObject, Program data, ServiceProvider serviceProvider) {
+		return rowObject.getProgramLocation();
 	}
 
 }

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symtable/SymbolTableModel.java

@@ -15,8 +15,7 @@
  */
 package ghidra.app.plugin.core.symtable;
 
-import java.util.LinkedList;
-import java.util.List;
+import java.util.*;
 
 import docking.widgets.table.*;
 import ghidra.app.cmd.function.DeleteFunctionCmd;
@@ -41,7 +40,12 @@ import ghidra.util.table.column.*;
 import ghidra.util.table.field.*;
 import ghidra.util.task.TaskMonitor;
 
-class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
+class SymbolTableModel extends AddressBasedTableModel<Symbol> {
+
+	private static final Comparator<Symbol> NAME_COL_COMPARATOR = (s1, s2) -> {
+		return s1.toString().compareToIgnoreCase(s2.toString());
+	};
+
 	static final int LABEL_COL = 0;
 	static final int LOCATION_COL = 1;
 	static final int TYPE_COL = 2;
@@ -62,17 +66,14 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 		this.provider = provider;
 		this.tool = tool;
 		this.filter = new NewSymbolFilter();
-
-		// leave off default sorting, as this can be slow; the user can sort as desired
-		setDefaultTableSortState(null);
 	}
 
 	@Override
-	protected TableColumnDescriptor<SymbolRowObject> createTableColumnDescriptor() {
-		TableColumnDescriptor<SymbolRowObject> descriptor = new TableColumnDescriptor<>();
+	protected TableColumnDescriptor<Symbol> createTableColumnDescriptor() {
+		TableColumnDescriptor<Symbol> descriptor = new TableColumnDescriptor<>();
 
-		descriptor.addVisibleColumn(new NameTableColumn(), 1, true);
-		descriptor.addVisibleColumn(new LocationTableColumn());
+		descriptor.addVisibleColumn(new NameTableColumn());
+		descriptor.addVisibleColumn(new LocationTableColumn(), 1, true);
 		descriptor.addVisibleColumn(new SymbolTypeTableColumn());
 		descriptor.addHiddenColumn(new DataTypeTableColumn());
 		descriptor.addVisibleColumn(new NamespaceTableColumn());
@@ -136,7 +137,7 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 	}
 
 	@Override
-	protected void doLoad(Accumulator<SymbolRowObject> accumulator, TaskMonitor monitor)
+	protected void doLoad(Accumulator<Symbol> accumulator, TaskMonitor monitor)
 			throws CancelledException {
 		if (symbolTable == null) {
 			return;
@@ -156,7 +157,7 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 			monitor.checkCanceled();
 			Symbol s = it.next();
 			if (filter.accepts(s, getProgram())) {
-				accumulator.add(new SymbolRowObject(s));
+				accumulator.add(s);
 			}
 		}
 		if (filter.acceptsDefaultLabelSymbols()) {
@@ -168,7 +169,7 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 				Address a = addrIt.next();
 				Symbol s = symbolTable.getPrimarySymbol(a);
 				if (s.isDynamic() && filter.accepts(s, getProgram())) {
-					accumulator.add(new SymbolRowObject(s));
+					accumulator.add(s);
 				}
 			}
 		}
@@ -193,8 +194,7 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 			return;
 		}
 
-		SymbolRowObject rowObject = filteredData.get(row);
-		Symbol symbol = symbolTable.getSymbol(rowObject.getKey());
+		Symbol symbol = filteredData.get(row);
 		if (symbol == null) {
 			return;
 		}
@@ -215,9 +215,9 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 
 	@Override
 	public ProgramLocation getProgramLocation(int row, int column) {
-		SymbolTableNameValue s = (SymbolTableNameValue) getValueAt(row, LABEL_COL);
+		Symbol s = (Symbol) getValueAt(row, LABEL_COL);
 		if (s != null) {
-			return s.getSymbol().getProgramLocation();
+			return s.getProgramLocation();
 		}
 		return null;
 	}
@@ -246,7 +246,7 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 
 	void symbolAdded(Symbol s) {
 		if (filter.accepts(s, getProgram())) {
-			addObject(new SymbolRowObject(s));
+			addObject(s);
 			lastSymbol = s;
 		}
 	}
@@ -255,27 +255,20 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 		if (lastSymbol != null && lastSymbol.getID() == s.getID()) {
 			lastSymbol = null;
 		}
-		removeObject(new SymbolRowObject(s));
-	}
-
-	void symbolRemoved(long symbolId) {
-		if (lastSymbol != null && lastSymbol.getID() == symbolId) {
-			lastSymbol = null;
-		}
-		removeObject(new SymbolRowObject(symbolId));
+		removeObject(s);
 	}
 
 	void symbolChanged(Symbol s) {
-		SymbolRowObject symbolRowObject = new SymbolRowObject(s);
+		Symbol Symbol = s;
 		if (filter.accepts(s, getProgram())) {
-			updateObject(symbolRowObject);
+			updateObject(Symbol);
 		}
 		else {
-			removeObject(symbolRowObject);
+			removeObject(Symbol);
 		}
 	}
 
-	void delete(List<SymbolRowObject> rowObjects) {
+	void delete(List<Symbol> rowObjects) {
 		if (rowObjects == null || rowObjects.size() == 0) {
 			return;
 		}
@@ -283,11 +276,7 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 		tool.setStatusInfo("");
 		List<Symbol> deleteList = new LinkedList<>();
 		CompoundCmd cmd = new CompoundCmd("Delete symbol(s)");
-		for (int i = 0; i < rowObjects.size(); i++) {
-			Symbol symbol = symbolTable.getSymbol(rowObjects.get(i).getKey());
-			if (symbol == null) {
-				continue;
-			}
+		for (Symbol symbol : rowObjects) {
 			if (symbol.isDynamic()) {
 				Symbol[] symbols = symbolTable.getSymbols(symbol.getAddress());
 				if (symbols.length == 1) {
@@ -296,7 +285,7 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 				}
 			}
 
-			deleteList.add(rowObjects.get(i).getSymbol());
+			deleteList.add(symbol);
 			String label = symbol.getName();
 			if (symbol.getSymbolType() == SymbolType.FUNCTION) {
 				Function function = (Function) symbol.getObject();
@@ -316,9 +305,10 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 		if (cmd.size() == 0) {
 			return;
 		}
+
 		if (tool.execute(cmd, getProgram())) {
-			for (int k = 0; k < deleteList.size(); k++) {
-				removeObject(new SymbolRowObject(deleteList.get(k)));
+			for (Symbol s : deleteList) {
+				removeObject(s);
 			}
 			updateNow();
 		}
@@ -334,15 +324,14 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 
 	@Override
 	public Address getAddress(int row) {
-		Symbol symbol = symbolTable.getSymbol(getRowObject(row).getKey());
+		Symbol symbol = getRowObject(row);
 		if (symbol == null) {
 			return null;
 		}
 		return symbol.getAddress();
 	}
 
-	private AddressBasedLocation getSymbolLocation(SymbolRowObject rowObject) {
-		Symbol s = rowObject.getSymbol();
+	private AddressBasedLocation getSymbolLocation(Symbol s) {
 		if (s == null) {
 			return new AddressBasedLocation();
 		}
@@ -350,17 +339,36 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 		if (type == SymbolType.PARAMETER || type == SymbolType.LOCAL_VAR) {
 			// Must use special location object for variables which renders variable storage
 			// location since this can't be obtained from just a variable storage address
-			return new VariableSymbolLocation((Variable) s.getObject());
+			Variable object = (Variable) s.getObject();
+			if (object == null) {
+				return null;
+			}
+			return new VariableSymbolLocation(object);
 		}
 		return new AddressBasedLocation(program, s.getAddress());
 	}
 
+	@Override
+	protected Comparator<Symbol> createSortComparator(int columnIndex) {
+		DynamicTableColumn<Symbol, ?, ?> column = getColumn(columnIndex);
+		if (column instanceof NameTableColumn) {
+			// note: we use our own name comparator to increase sorting speed for the name 
+			//       column.  This works because this comparator is called for each *row object* 
+			//       allowing the comparator to compare the Symbols based on name instead of 
+			//       having to use the table model's code for getting a column value for the
+			//       row object.   The code for retrieving a column value is slower than just
+			//       working with the row object directly.  See 
+			//       ThreadedTableModel.getCachedColumnValueForRow for more info.
+			return NAME_COL_COMPARATOR;
+		}
+		return super.createSortComparator(columnIndex);
+	}
+
 //==================================================================================================
 // Table Column Classes
 //==================================================================================================
 
-	private class NameTableColumn
-			extends AbstractProgramBasedDynamicTableColumn<SymbolRowObject, SymbolTableNameValue> {
+	private class NameTableColumn extends AbstractProgramBasedDynamicTableColumn<Symbol, Symbol> {
 
 		@Override
 		public String getColumnName() {
@@ -368,25 +376,18 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 		}
 
 		@Override
-		public SymbolTableNameValue getValue(SymbolRowObject rowObject, Settings settings,
-				Program p, ServiceProvider svcProvider) throws IllegalArgumentException {
+		public Symbol getValue(Symbol symbol, Settings settings, Program p,
+				ServiceProvider svcProvider) throws IllegalArgumentException {
 
-			Symbol s = rowObject.getSymbol();
-			if (s == null) {
+			if (!symbol.checkIsValid()) {
 				return null;
 			}
-
-			// Note: this call is slow, especially for dynamic symbols.  Caching the dynamic 
-			// symbols in the SymbolRowObject *greatly* increases sorting and filtering performance.
-			// For now we assume that most users are not loading dynamic labels.  If we add 
-			// caching, then we have to deal with the stickiness of when to clear/update the cache
-			String name = s.toString();
-			return new SymbolTableNameValue(s, name);
+			return symbol;
 		}
 	}
 
 	private class PinnedTableColumn
-			extends AbstractProgramBasedDynamicTableColumn<SymbolRowObject, Boolean> {
+			extends AbstractProgramBasedDynamicTableColumn<Symbol, Boolean> {
 
 		private PinnedRenderer renderer = new PinnedRenderer();
 
@@ -396,10 +397,10 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 		}
 
 		@Override
-		public Boolean getValue(SymbolRowObject rowObject, Settings settings, Program p,
+		public Boolean getValue(Symbol symbol, Settings settings, Program p,
 				ServiceProvider svcProvider) throws IllegalArgumentException {
-			Symbol symbol = rowObject.getSymbol();
-			if (symbol == null) {
+
+			if (!symbol.checkIsValid()) {
 				return null;
 			}
 			return symbol.isPinned();
@@ -417,7 +418,7 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 	}
 
 	private class LocationTableColumn
-			extends AbstractProgramLocationTableColumn<SymbolRowObject, AddressBasedLocation> {
+			extends AbstractProgramLocationTableColumn<Symbol, AddressBasedLocation> {
 
 		@Override
 		public String getColumnName() {
@@ -425,16 +426,16 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 		}
 
 		@Override
-		public AddressBasedLocation getValue(SymbolRowObject rowObject, Settings settings,
-				Program p, ServiceProvider svcProvider) throws IllegalArgumentException {
-			return getSymbolLocation(rowObject);
+		public AddressBasedLocation getValue(Symbol symbol, Settings settings, Program p,
+				ServiceProvider svcProvider) throws IllegalArgumentException {
+			return getSymbolLocation(symbol);
 		}
 
 		@Override
-		public ProgramLocation getProgramLocation(SymbolRowObject rowObject, Settings settings,
-				Program p, ServiceProvider svcProvider) {
-			Symbol symbol = rowObject.getSymbol();
-			if (symbol == null) {
+		public ProgramLocation getProgramLocation(Symbol symbol, Settings settings, Program p,
+				ServiceProvider svcProvider) {
+
+			if (!symbol.checkIsValid()) {
 				return null;
 			}
 			return symbol.getProgramLocation();
@@ -442,7 +443,7 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 	}
 
 	private class SymbolTypeTableColumn
-			extends AbstractProgramBasedDynamicTableColumn<SymbolRowObject, String> {
+			extends AbstractProgramBasedDynamicTableColumn<Symbol, String> {
 
 		@Override
 		public String getColumnName() {
@@ -450,17 +451,16 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 		}
 
 		@Override
-		public String getValue(SymbolRowObject rowObject, Settings settings, Program p,
+		public String getValue(Symbol symbol, Settings settings, Program p,
 				ServiceProvider svcProvider) throws IllegalArgumentException {
 
-			Symbol s = rowObject.getSymbol();
-			if (s == null) {
+			if (!symbol.checkIsValid()) {
 				return null;
 			}
 
 			// Note: this call is slow.  If we decide that filtering/sorting on this value is
 			//       important, then this should be cached
-			return SymbolUtilities.getSymbolTypeDisplayName(s);
+			return SymbolUtilities.getSymbolTypeDisplayName(symbol);
 		}
 	}
 
@@ -472,7 +472,7 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 	}
 
 	private class DataTypeTableColumn
-			extends AbstractProgramBasedDynamicTableColumn<SymbolRowObject, String> {
+			extends AbstractProgramBasedDynamicTableColumn<Symbol, String> {
 
 		@Override
 		public String getColumnName() {
@@ -480,11 +480,10 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 		}
 
 		@Override
-		public String getValue(SymbolRowObject rowObject, Settings settings, Program p,
+		public String getValue(Symbol symbol, Settings settings, Program p,
 				ServiceProvider svcProvider) throws IllegalArgumentException {
 
-			Symbol symbol = rowObject.getSymbol();
-			if (symbol == null) {
+			if (!symbol.checkIsValid()) {
 				return null;
 			}
 
@@ -510,7 +509,7 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 	}
 
 	private class NamespaceTableColumn
-			extends AbstractProgramBasedDynamicTableColumn<SymbolRowObject, String> {
+			extends AbstractProgramBasedDynamicTableColumn<Symbol, String> {
 
 		@Override
 		public String getColumnName() {
@@ -518,19 +517,18 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 		}
 
 		@Override
-		public String getValue(SymbolRowObject rowObject, Settings settings, Program p,
+		public String getValue(Symbol symbol, Settings settings, Program p,
 				ServiceProvider svcProvider) throws IllegalArgumentException {
-			Symbol symbol = rowObject.getSymbol();
-			if (symbol == null) {
+
+			if (!symbol.checkIsValid()) {
 				return null;
 			}
-
 			return symbol.getParentNamespace().getName(true);
 		}
 	}
 
 	private class SourceTableColumn
-			extends AbstractProgramBasedDynamicTableColumn<SymbolRowObject, SourceType> {
+			extends AbstractProgramBasedDynamicTableColumn<Symbol, SourceType> {
 
 		private GColumnRenderer<SourceType> renderer = new AbstractGColumnRenderer<SourceType>() {
 			@Override
@@ -558,9 +556,9 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 		}
 
 		@Override
-		public SourceType getValue(SymbolRowObject rowObject, Settings settings, Program p,
+		public SourceType getValue(Symbol symbol, Settings settings, Program p,
 				ServiceProvider svcProvider) throws IllegalArgumentException {
-			Symbol symbol = rowObject.getSymbol();
+
 			if (symbol == null) {
 				return null;
 			}
@@ -570,7 +568,7 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 	}
 
 	private class ReferenceCountTableColumn
-			extends AbstractProgramBasedDynamicTableColumn<SymbolRowObject, Integer> {
+			extends AbstractProgramBasedDynamicTableColumn<Symbol, Integer> {
 
 		private ReferenceCountRenderer renderer = new ReferenceCountRenderer();
 
@@ -580,13 +578,11 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 		}
 
 		@Override
-		public Integer getValue(SymbolRowObject rowObject, Settings settings, Program p,
+		public Integer getValue(Symbol symbol, Settings settings, Program p,
 				ServiceProvider svcProvider) throws IllegalArgumentException {
-			Symbol symbol = rowObject.getSymbol();
-			if (symbol == null) {
+			if (!symbol.checkIsValid()) {
 				return null;
 			}
-
 			return Integer.valueOf(symbol.getReferenceCount());
 		}
 
@@ -604,7 +600,7 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 	}
 
 	private class OffcutReferenceCountTableColumn
-			extends AbstractProgramBasedDynamicTableColumn<SymbolRowObject, Integer> {
+			extends AbstractProgramBasedDynamicTableColumn<Symbol, Integer> {
 
 		private OffcutReferenceCountRenderer renderer = new OffcutReferenceCountRenderer();
 
@@ -614,11 +610,9 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 		}
 
 		@Override
-		public Integer getValue(SymbolRowObject rowObject, Settings settings, Program p,
+		public Integer getValue(Symbol symbol, Settings settings, Program p,
 				ServiceProvider svcProvider) throws IllegalArgumentException {
-
-			Symbol symbol = rowObject.getSymbol();
-			if (symbol == null) {
+			if (!symbol.checkIsValid()) {
 				return null;
 			}
 
@@ -655,8 +649,7 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 		}
 	}
 
-	private class UserTableColumn
-			extends AbstractProgramBasedDynamicTableColumn<SymbolRowObject, String> {
+	private class UserTableColumn extends AbstractProgramBasedDynamicTableColumn<Symbol, String> {
 
 		@Override
 		public String getColumnName() {
@@ -669,11 +662,10 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 		}
 
 		@Override
-		public String getValue(SymbolRowObject rowObject, Settings settings, Program p,
+		public String getValue(Symbol symbol, Settings settings, Program p,
 				ServiceProvider svcProvider) throws IllegalArgumentException {
 
-			Symbol symbol = rowObject.getSymbol();
-			if (symbol == null) {
+			if (!symbol.checkIsValid()) {
 				return null;
 			}
 
@@ -694,7 +686,7 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 	}
 
 	private class OriginalNameColumn
-			extends AbstractProgramBasedDynamicTableColumn<SymbolRowObject, String> {
+			extends AbstractProgramBasedDynamicTableColumn<Symbol, String> {
 
 		@Override
 		public String getColumnName() {
@@ -707,13 +699,17 @@ class SymbolTableModel extends AddressBasedTableModel<SymbolRowObject> {
 		}
 
 		@Override
-		public String getValue(SymbolRowObject rowObject, Settings settings, Program p,
+		public String getValue(Symbol symbol, Settings settings, Program p,
 				ServiceProvider svcProvider) throws IllegalArgumentException {
 
-			Symbol symbol = rowObject.getSymbol();
-			if (symbol == null || !symbol.isExternal()) {
+			if (!symbol.checkIsValid()) {
 				return null;
 			}
+
+			if (!symbol.isExternal()) {
+				return null;
+			}
+
 			SymbolType symbolType = symbol.getSymbolType();
 			if (symbolType != SymbolType.FUNCTION && symbolType != SymbolType.LABEL) {
 				return null;

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symtable/SymbolTableNameValue.java

@@ -1,52 +0,0 @@
-/* ###
- * 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 ghidra.program.model.symbol.Symbol;
-
-/**
- * A simple data object for the Name column table cell.  This class allows us to control
- * how sorting is performed by caching the slow (potentially) to calculate symbol name.
- */
-class SymbolTableNameValue implements Comparable<SymbolTableNameValue> {
-
-	private Symbol symbol;
-	private String name;
-
-	SymbolTableNameValue(Symbol symbol, String name) {
-		this.symbol = symbol;
-		this.name = name;
-
-		// name will be non-null when cached by the table model
-		if (name == null) {
-			name = symbol.toString();
-		}
-	}
-
-	Symbol getSymbol() {
-		return symbol;
-	}
-
-	@Override
-	public int compareTo(SymbolTableNameValue o) {
-		return name.compareToIgnoreCase(o.name);
-	}
-
-	@Override
-	public String toString() {
-		return name;
-	}
-}

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symtable/SymbolTablePlugin.java

@@ -37,8 +37,7 @@ import ghidra.framework.plugintool.util.PluginStatus;
 import ghidra.program.model.address.Address;
 import ghidra.program.model.listing.Data;
 import ghidra.program.model.listing.Program;
-import ghidra.program.model.symbol.Reference;
-import ghidra.program.model.symbol.Symbol;
+import ghidra.program.model.symbol.*;
 import ghidra.program.util.ChangeManager;
 import ghidra.program.util.ProgramChangeRecord;
 import ghidra.util.table.GhidraTable;
@@ -210,11 +209,12 @@ public class SymbolTablePlugin extends Plugin implements DomainObjectListener {
 
 			ProgramChangeRecord rec = (ProgramChangeRecord) doRecord;
 			Symbol symbol = null;
+			SymbolTable symbolTable = currentProgram.getSymbolTable();
 			switch (eventType) {
 				case ChangeManager.DOCR_CODE_ADDED:
 				case ChangeManager.DOCR_CODE_REMOVED:
 					if (rec.getNewValue() instanceof Data) {
-						symbol = currentProgram.getSymbolTable().getPrimarySymbol(rec.getStart());
+						symbol = symbolTable.getPrimarySymbol(rec.getStart());
 						if (symbol != null && symbol.isDynamic()) {
 							symProvider.symbolChanged(symbol);
 							refProvider.symbolChanged(symbol);
@@ -224,7 +224,7 @@ public class SymbolTablePlugin extends Plugin implements DomainObjectListener {
 
 				case ChangeManager.DOCR_SYMBOL_ADDED:
 					Address addAddr = rec.getStart();
-					Symbol primaryAtAdd = currentProgram.getSymbolTable().getPrimarySymbol(addAddr);
+					Symbol primaryAtAdd = symbolTable.getPrimarySymbol(addAddr);
 					if (primaryAtAdd != null && primaryAtAdd.isDynamic()) {
 						symProvider.symbolRemoved(primaryAtAdd);
 					}
@@ -236,10 +236,11 @@ public class SymbolTablePlugin extends Plugin implements DomainObjectListener {
 				case ChangeManager.DOCR_SYMBOL_REMOVED:
 					Address removeAddr = rec.getStart();
 					Long symbolID = (Long) rec.getNewValue();
-					symProvider.symbolRemoved(symbolID.longValue());
-					refProvider.symbolRemoved(symbolID.longValue());
-					Symbol primaryAtRemove =
-						currentProgram.getSymbolTable().getPrimarySymbol(removeAddr);
+					Symbol removedSymbol =
+						symbolTable.createSymbolPlaceholder(removeAddr, symbolID);
+					symProvider.symbolRemoved(removedSymbol);
+					refProvider.symbolRemoved(removedSymbol);
+					Symbol primaryAtRemove = symbolTable.getPrimarySymbol(removeAddr);
 					if (primaryAtRemove != null && primaryAtRemove.isDynamic()) {
 						symProvider.symbolAdded(primaryAtRemove);
 					}
@@ -274,7 +275,7 @@ public class SymbolTablePlugin extends Plugin implements DomainObjectListener {
 					break;
 				case ChangeManager.DOCR_MEM_REFERENCE_ADDED:
 					Reference ref = (Reference) rec.getObject();
-					symbol = currentProgram.getSymbolTable().getSymbol(ref);
+					symbol = symbolTable.getSymbol(ref);
 					if (symbol != null) {
 						symProvider.symbolChanged(symbol);
 						refProvider.symbolChanged(symbol);
@@ -284,11 +285,12 @@ public class SymbolTablePlugin extends Plugin implements DomainObjectListener {
 					ref = (Reference) rec.getObject();
 					Address toAddr = ref.getToAddress();
 					if (toAddr.isMemoryAddress()) {
-						symbol = currentProgram.getSymbolTable().getSymbol(ref);
+						symbol = symbolTable.getSymbol(ref);
 						if (symbol == null) {
-							long id = currentProgram.getSymbolTable().getDynamicSymbolID(
-								ref.getToAddress());
-							symProvider.symbolRemoved(id);
+
+							long id = symbolTable.getDynamicSymbolID(ref.getToAddress());
+							removedSymbol = symbolTable.createSymbolPlaceholder(toAddr, id);
+							symProvider.symbolRemoved(removedSymbol);
 						}
 						else {
 							refProvider.symbolChanged(symbol);
@@ -298,7 +300,7 @@ public class SymbolTablePlugin extends Plugin implements DomainObjectListener {
 
 				case ChangeManager.DOCR_EXTERNAL_ENTRY_POINT_ADDED:
 				case ChangeManager.DOCR_EXTERNAL_ENTRY_POINT_REMOVED:
-					Symbol[] symbols = currentProgram.getSymbolTable().getSymbols(rec.getStart());
+					Symbol[] symbols = symbolTable.getSymbols(rec.getStart());
 					for (Symbol element : symbols) {
 						symProvider.symbolChanged(element);
 						refProvider.symbolChanged(element);

Ghidra/Features/Base/src/main/java/ghidra/test/AbstractGhidraHeadlessIntegrationTest.java

@@ -47,7 +47,8 @@ import ghidra.util.exception.AssertException;
 import ghidra.util.exception.RollbackException;
 import junit.framework.AssertionFailedError;
 import utility.application.ApplicationLayout;
-import utility.function.*;
+import utility.function.ExceptionalCallback;
+import utility.function.ExceptionalFunction;
 
 public abstract class AbstractGhidraHeadlessIntegrationTest extends AbstractDockingTest {
 
@@ -107,7 +108,7 @@ public abstract class AbstractGhidraHeadlessIntegrationTest extends AbstractDock
 	 * if found.  If no language is found, an exception will be thrown.
 	 * @param oldLanguageName old language name string
 	 * @return the language compiler and spec
-	 * @throws LanguageNotFoundException 
+	 * @throws LanguageNotFoundException if the language is not found
 	 */
 	public static LanguageCompilerSpecPair getLanguageCompilerSpecPair(String oldLanguageName)
 			throws LanguageNotFoundException {
@@ -194,7 +195,14 @@ public abstract class AbstractGhidraHeadlessIntegrationTest extends AbstractDock
 		}
 	}
 
-	public static <E extends Exception> void tx(Program p, ExceptionalCallback<E> c) throws E {
+	/**
+	 * Provides a convenient method for modifying the current program, handling the transaction
+	 * logic. 
+	 * 
+	 * @param p the program
+	 * @param c the code to execute
+	 */
+	public static <E extends Exception> void tx(Program p, ExceptionalCallback<E> c) {
 		int txId = p.startTransaction("Test - Function in Transaction");
 		boolean commit = true;
 		try {
@@ -202,9 +210,9 @@ public abstract class AbstractGhidraHeadlessIntegrationTest extends AbstractDock
 			p.flushEvents();
 			waitForSwing();
 		}
-		catch (RollbackException e) {
+		catch (Exception e) {
 			commit = false;
-			throw e;
+			failWithException("Exception modifying program '" + p.getName() + "'", e);
 		}
 		finally {
 			p.endTransaction(txId, commit);
@@ -213,27 +221,14 @@ public abstract class AbstractGhidraHeadlessIntegrationTest extends AbstractDock
 
 	/**
 	 * Provides a convenient method for modifying the current program, handling the transaction
-	 * logic 
+	 * logic.   This method is calls {@link #tx(Program, ExceptionalCallback)}, but helps with
+	 * semantics.
 	 * 
-	 * @param program the program
-	 * @param callback the code to execute
+	 * @param p the program
+	 * @param c the code to execute
 	 */
-	public <E extends Exception> void modifyProgram(Program program,
-			ExceptionalConsumer<Program, E> callback) {
-		assertNotNull("Program cannot be null", program);
-
-		boolean commit = false;
-		int tx = program.startTransaction("Test");
-		try {
-			callback.accept(program);
-			commit = true;
-		}
-		catch (Exception e) {
-			failWithException("Exception modifying program '" + program.getName() + "'", e);
-		}
-		finally {
-			program.endTransaction(tx, commit);
-		}
+	public static <E extends Exception> void modifyProgram(Program p, ExceptionalCallback<E> c) {
+		tx(p, c);
 	}
 
 	/**
@@ -244,7 +239,7 @@ public abstract class AbstractGhidraHeadlessIntegrationTest extends AbstractDock
 	 * @param f the function for modifying the program and creating the desired result
 	 * @return the result
 	 */
-	public <R, E extends Exception> R createInProgram(Program program,
+	public <R, E extends Exception> R modifyProgram(Program program,
 			ExceptionalFunction<Program, R, E> f) {
 		assertNotNull("Program cannot be null", program);
 

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/AbstractSortedTableModel.java

@@ -20,8 +20,7 @@ import java.util.*;
 import javax.swing.event.TableModelEvent;
 import javax.swing.table.TableModel;
 
-import docking.widgets.table.sort.DefaultColumnComparator;
-import docking.widgets.table.sort.RowToColumnComparator;
+import docking.widgets.table.sort.*;
 import ghidra.util.Swing;
 import ghidra.util.datastruct.WeakDataStructureFactory;
 import ghidra.util.datastruct.WeakSet;
@@ -331,7 +330,7 @@ public abstract class AbstractSortedTableModel<T> extends AbstractGTableModel<T>
 	 */
 	protected Comparator<T> createSortComparator(int columnIndex) {
 		return new RowToColumnComparator<>(this, columnIndex, new DefaultColumnComparator(),
-			new StringBasedBackupRowToColumnComparator(columnIndex));
+			new StringBasedBackupRowToColumnComparator());
 	}
 
 	private Comparator<T> createLastResortComparator(ComparatorLink parentChain) {
@@ -470,22 +469,16 @@ public abstract class AbstractSortedTableModel<T> extends AbstractGTableModel<T>
 		}
 	}
 
-	private class StringBasedBackupRowToColumnComparator implements Comparator<T> {
-
-		private int sortColumn;
-
-		StringBasedBackupRowToColumnComparator(int sortColumn) {
-			this.sortColumn = sortColumn;
-		}
+	private class StringBasedBackupRowToColumnComparator implements BackupColumnComparator<T> {
 
 		@Override
-		public int compare(T t1, T t2) {
+		public int compare(T t1, T t2, Object c1, Object c2) {
 			if (t1 == t2) {
 				return 0;
 			}
 
-			String s1 = getColumStringValue(t1);
-			String s2 = getColumStringValue(t2);
+			String s1 = getColumStringValue(c1);
+			String s2 = getColumStringValue(c2);
 
 			if (s1 == null || s2 == null) {
 				return TableComparators.compareWithNullValues(s1, s2);
@@ -494,11 +487,10 @@ public abstract class AbstractSortedTableModel<T> extends AbstractGTableModel<T>
 			return s1.compareToIgnoreCase(s2);
 		}
 
-		private String getColumStringValue(T t) {
+		private String getColumStringValue(Object columnValue) {
 			// just use the toString(), which may or may not produce a good value (this will
 			// catch the cases where the column value is itself a string)
-			Object o = getColumnValueForRow(t, sortColumn);
-			return o == null ? null : o.toString();
+			return columnValue == null ? null : columnValue.toString();
 		}
 	}
 }

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/AddRemoveListItem.java

@@ -1,6 +1,5 @@
 /* ###
  * IP: GHIDRA
- * REVIEWED: YES
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -34,6 +33,7 @@ public class AddRemoveListItem<T> {
 	public boolean isRemove() {
 		return isRemove;
 	}
+
 	public boolean isChange() {
 		return isAdd && isRemove;
 	}
@@ -41,4 +41,16 @@ public class AddRemoveListItem<T> {
 	public T getValue() {
 		return value;
 	}
+
+	@Override
+	public String toString() {
+
+		//@formatter:off
+		return "{\n" + 
+			"\tvalue: " + value +",\n" +
+			"\tisAdd: " + isAdd +",\n" +
+			"\tisRemove: " + isRemove +"\n" +
+		"}";
+		//@formatter:on
+	}
 }

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/GDynamicColumnTableModel.java

@@ -441,7 +441,6 @@ public abstract class GDynamicColumnTableModel<ROW_TYPE, DATA_SOURCE>
 		}
 
 		return column.getValue(t, columnSettings.get(column), dataSource, serviceProvider);
-
 	}
 
 	/**

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/sort/BackupColumnComparator.java

@@ -0,0 +1,50 @@
+/* ###
+ * 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.sort;
+
+import java.util.Comparator;
+
+/**
+ * An interface that is conceptually the same as a {@link Comparator}.  The only difference is 
+ * that we pass the row objects <b>and</b> the column values to 
+ * {@link #compare(Object, Object, Object, Object)}.  This allows us to take advantage of 
+ * already-retrieved column values.  This can speed-up table sorting, as repeatedly retrieving
+ * column values for each comparison is slow.
+ *
+ * @param <T> the row type
+ */
+public interface BackupColumnComparator<T> {
+
+	static final BackupColumnComparator<Object> NO_SORT_COMPARATOR = (t1, t2, o1, o2) -> 0;
+
+	@SuppressWarnings("unchecked") // we are casting to Object; safe since no comparisons are done
+	public static <T> BackupColumnComparator<T> getNoSortComparator() {
+		return (BackupColumnComparator<T>) NO_SORT_COMPARATOR;
+	}
+
+	/**
+	 * Compares two row/column values using the same contract as 
+	 * {@link Comparator#compare(Object, Object)}
+	 * 
+	 * @param t1 the 1st row object
+	 * @param t2 the 2nd row object
+	 * @param c1 the 1st column value
+	 * @param c2 the second column value
+	 * @return 0 if the 2 values compare the same; negative if the first value compares less than
+	 *         the second; positive if the first value compares as larger than the first
+	 */
+	public int compare(T t1, T t2, Object c1, Object c2);
+}

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/sort/ColumnRenderedValueBackupRowComparator.java

@@ -15,8 +15,6 @@
  */
 package docking.widgets.table.sort;
 
-import java.util.Comparator;
-
 import docking.widgets.table.*;
 import ghidra.docking.settings.Settings;
 import ghidra.util.table.column.GColumnRenderer;
@@ -29,7 +27,7 @@ import ghidra.util.table.column.GColumnRenderer.ColumnConstraintFilterMode;
  * 
  * @param <T> the row type 
  */
-public class ColumnRenderedValueBackupRowComparator<T> implements Comparator<T> {
+public class ColumnRenderedValueBackupRowComparator<T> implements BackupColumnComparator<T> {
 
 	protected int sortColumn;
 	protected DynamicColumnTableModel<T> model;
@@ -56,13 +54,13 @@ public class ColumnRenderedValueBackupRowComparator<T> implements Comparator<T>
 	}
 
 	@Override
-	public int compare(T t1, T t2) {
+	public int compare(T t1, T t2, Object c1, Object c2) {
 		if (t1 == t2) {
 			return 0;
 		}
 
-		String s1 = getRenderedColumnStringValue(t1);
-		String s2 = getRenderedColumnStringValue(t2);
+		String s1 = getRenderedColumnStringValue(c1);
+		String s2 = getRenderedColumnStringValue(c2);
 
 		if (s1 == null || s2 == null) {
 			return TableComparators.compareWithNullValues(s1, s2);
@@ -75,7 +73,7 @@ public class ColumnRenderedValueBackupRowComparator<T> implements Comparator<T>
 	// unsafe.  We happen know that we retrieved the value from the column that we are passing
 	// it to, so the casting and usage is indeed safe.
 	@SuppressWarnings("unchecked")
-	private String getRenderedColumnStringValue(T t) {
+	private String getRenderedColumnStringValue(Object columnValue) {
 
 		if (!supportsColumnSorting) {
 			return null;
@@ -83,13 +81,12 @@ public class ColumnRenderedValueBackupRowComparator<T> implements Comparator<T>
 
 		DynamicTableColumn<T, ?, ?> column = model.getColumn(sortColumn);
 		GColumnRenderer<Object> renderer = (GColumnRenderer<Object>) column.getColumnRenderer();
-		Object o = getColumnValue(t);
 		if (renderer == null) {
-			return o == null ? null : o.toString();
+			return columnValue == null ? null : columnValue.toString();
 		}
 
 		Settings settings = model.getColumnSettings(sortColumn);
-		return renderer.getFilterString(o, settings);
+		return renderer.getFilterString(columnValue, settings);
 	}
 
 	// this may be overridden to use caching

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/sort/RowToColumnComparator.java

@@ -32,7 +32,8 @@ public class RowToColumnComparator<T> implements Comparator<T> {
 	protected RowObjectTableModel<T> model;
 	protected int sortColumn;
 	protected Comparator<Object> columnComparator;
-	protected Comparator<T> backupRowComparator = TableComparators.getNoSortComparator();
+	protected BackupColumnComparator<T> backupRowComparator =
+		BackupColumnComparator.getNoSortComparator();
 
 	/**
 	 * Constructs this class with the given column comparator that will get called after the
@@ -60,7 +61,7 @@ public class RowToColumnComparator<T> implements Comparator<T> {
 	 * @param backupRowComparator the backup row comparator
 	 */
 	public RowToColumnComparator(RowObjectTableModel<T> model, int sortColumn,
-			Comparator<Object> comparator, Comparator<T> backupRowComparator) {
+			Comparator<Object> comparator, BackupColumnComparator<T> backupRowComparator) {
 		this.model = model;
 		this.sortColumn = sortColumn;
 		this.columnComparator = Objects.requireNonNull(comparator);
@@ -97,7 +98,7 @@ public class RowToColumnComparator<T> implements Comparator<T> {
 		// backup comparator is not a stub and will do something reasonable for the sort, 
 		// depending upon how the model created this class.
 		//
-		return backupRowComparator.compare(t1, t2);
+		return backupRowComparator.compare(t1, t2, value1, value2);
 	}
 
 	protected Object getColumnValue(T t) {

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/ThreadedTableColumnComparator.java

@@ -17,6 +17,7 @@ package docking.widgets.table.threaded;
 
 import java.util.Comparator;
 
+import docking.widgets.table.sort.BackupColumnComparator;
 import docking.widgets.table.sort.RowToColumnComparator;
 
 /**
@@ -55,7 +56,7 @@ public class ThreadedTableColumnComparator<T> extends RowToColumnComparator<T> {
 	 * @see RowToColumnComparator
 	 */
 	public ThreadedTableColumnComparator(ThreadedTableModel<T, ?> model, int sortColumn,
-			Comparator<Object> comparator, Comparator<T> backupRowComparator) {
+			Comparator<Object> comparator, BackupColumnComparator<T> backupRowComparator) {
 		super(model, sortColumn, comparator, backupRowComparator);
 		this.threadedModel = model;
 	}

Ghidra/Framework/Docking/src/main/java/docking/widgets/table/threaded/ThreadedTableModel.java

@@ -214,7 +214,37 @@ public abstract class ThreadedTableModel<ROW_OBJECT, DATA_SOURCE>
 	protected abstract void doLoad(Accumulator<ROW_OBJECT> accumulator, TaskMonitor monitor)
 			throws CancelledException;
 
+	/**
+	 * This method will retrieve a column value for the given row object.  Further, the retrieved
+	 * value will be cached.   This is useful when sorting a table, as the same column value may
+	 * be requested multiple times.
+	 * 
+	 * <p><u>Performance Notes</u>
+	 * <ul>
+	 * 	<li>This method uses a {@link HashMap} to cache column values for a row object.   Further,
+	 *      upon a key collision, the map will perform O(logn) lookups <b>if the 
+	 *      key (the row object) is {@link Comparable}</b>.   If the key is not comparable, then
+	 *      the collision lookups will be linear.    So, make your row objects comparable
+	 *      for maximum speed <b>when your table size becomes large</b>  (for small tables there
+	 *      is no observable impact).
+	 *  <li>Even if your row objects are comparable, relying on this table model to convert your 
+	 *      row object into column values can be slow <b>for large tables</b>.  This is because
+	 *      the default column comparison framework for the tables will call this method 
+	 *      multiple times, resulting in many more method calls per column value lookup.  For 
+	 *      large data, the repeated method calls start to become noticeable.  For maximum 
+	 *      column sorting speed, use a comparator that works not on the column value, but on 
+	 *      the row value.  To do this, return a comparator from your model's 
+	 *      {@link #createSortComparator(int)} method, instead of from the column itself or 
+	 *      by relying on column item implementing {@link Comparable}.  This is possible any
+	 *      time that a row object already has a field that is used for a given column.
+	 * </ul>
+	 * 
+	 * @param rowObject the row object
+	 * @param columnIndex the column index for which to get a value
+	 * @return the column value
+	 */
 	Object getCachedColumnValueForRow(ROW_OBJECT rowObject, int columnIndex) {
+
 		Map<ROW_OBJECT, Map<Integer, Object>> cachedColumnValues = threadLocalColumnCache.get();
 
 		if (cachedColumnValues == null) {

Ghidra/Framework/Generic/src/main/java/generic/test/TestThread.java

@@ -58,7 +58,7 @@ public class TestThread extends Thread {
 	/**
 	 * Returns true if the given thread name is the test thread name
 	 * 
-	 * @param t the thread name to check
+	 * @param name the thread name to check
 	 * @return true if the given thread name is the test thread name
 	 */
 	public static boolean isTestThreadName(String name) {

Ghidra/Framework/Generic/src/main/java/ghidra/util/datastruct/LRUMap.java

@@ -48,7 +48,7 @@ public class LRUMap<K, V> implements Map<K, V> {
 	protected HashMap<K, Entry<K, V>> map;
 	private int cacheSize;
 	private Entry<K, V> head;
-	private long modificationID = 0;
+	private volatile long modificationID = 0;
 
 	public LRUMap(int cacheSize) {
 		this.cacheSize = cacheSize;
@@ -267,7 +267,7 @@ public class LRUMap<K, V> implements Map<K, V> {
 
 	/**
 	 * This is called after an item has been removed from the cache.
-	 * @param eldest the ite being removed
+	 * @param eldest the item being removed
 	 */
 	protected void eldestEntryRemoved(Map.Entry<K, V> eldest) {
 		// this is just a way for subclasses to know when items are removed from the cache

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/symbol/CodeSymbol.java

@@ -217,7 +217,8 @@ public class CodeSymbol extends SymbolDB {
 	 */
 	@Override
 	public boolean isValidParent(Namespace parent) {
-		return SymbolType.LABEL.isValidParent(symbolMgr.getProgram(), parent, address, isExternal());
+		return SymbolType.LABEL.isValidParent(symbolMgr.getProgram(), parent, address,
+			isExternal());
 
 //		if (isExternal() != parent.isExternal()) {
 //			return false;
@@ -241,15 +242,12 @@ public class CodeSymbol extends SymbolDB {
 //		return true;
 	}
 
-	/**
-	 * @see ghidra.program.model.symbol.Symbol#getName()
-	 */
 	@Override
-	public String getName() {
+	protected String doGetName() {
 		if (getSource() == SourceType.DEFAULT && isExternal()) {
 			return ExternalManagerDB.getDefaultExternalName(this);
 		}
-		return super.getName();
+		return super.doGetName();
 	}
 
 	@Override

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/symbol/FunctionSymbol.java

@@ -33,7 +33,6 @@ import ghidra.util.Msg;
 import ghidra.util.exception.DuplicateNameException;
 import ghidra.util.exception.InvalidInputException;
 import ghidra.util.task.TaskMonitor;
-import ghidra.util.task.TaskMonitorAdapter;
 
 /**
  * Symbol class for functions.
@@ -237,11 +236,8 @@ public class FunctionSymbol extends SymbolDB {
 			isExternal());
 	}
 
-	/**
-	 * @see ghidra.program.model.symbol.Symbol#getName()
-	 */
 	@Override
-	public String getName() {
+	protected String doGetName() {
 		if (getSource() == SourceType.DEFAULT) {
 			if (isExternal()) {
 				return ExternalManagerDB.getDefaultExternalName(this);
@@ -251,17 +247,17 @@ public class FunctionSymbol extends SymbolDB {
 			Symbol thunkedSymbol = getThunkedSymbol();
 			if (thunkedSymbol instanceof FunctionSymbol) {
 				FunctionSymbol thunkedFuncSym = (FunctionSymbol) thunkedSymbol;
-				String name = thunkedFuncSym.getName();
+				String thunkName = thunkedFuncSym.getName();
 				if (thunkedFuncSym.getSource() == SourceType.DEFAULT &&
 					thunkedFuncSym.getThunkedSymbol() == null) {
 					// if thunking a default non-thunk function
-					name = "thunk_" + name;
+					thunkName = "thunk_" + thunkName;
 				}
-				return name;
+				return thunkName;
 			}
 			return SymbolUtilities.getDefaultFunctionName(address);
 		}
-		return super.getName();
+		return super.doGetName();
 	}
 
 //	@Override
@@ -363,7 +359,7 @@ public class FunctionSymbol extends SymbolDB {
 			checkIsValid();
 			Reference[] refs = super.getReferences(monitor);
 			if (monitor == null) {
-				monitor = TaskMonitorAdapter.DUMMY_MONITOR;
+				monitor = TaskMonitor.DUMMY;
 			}
 			if (monitor.isCancelled()) {
 				return refs;

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/symbol/GlobalVariableSymbolDB.java

@@ -54,7 +54,7 @@ public class GlobalVariableSymbolDB extends VariableSymbolDB {
 	}
 
 	@Override
-	public String getName() {
+	protected String doGetName() {
 		if (!checkIsValid()) {
 			// TODO: SCR 
 			return "[Invalid VariableSymbol - Deleted!]";
@@ -63,7 +63,7 @@ public class GlobalVariableSymbolDB extends VariableSymbolDB {
 		if (storage == null) {
 			return Function.DEFAULT_LOCAL_PREFIX + "_!BAD!";
 		}
-		return super.getName();
+		return super.doGetName();
 	}
 
 }

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/symbol/SymbolDB.java

@@ -30,11 +30,13 @@ import ghidra.program.model.listing.CircularDependencyException;
 import ghidra.program.model.listing.Program;
 import ghidra.program.model.symbol.*;
 import ghidra.program.util.ChangeManager;
+import ghidra.program.util.ProgramLocation;
 import ghidra.util.Lock;
 import ghidra.util.SystemUtilities;
 import ghidra.util.exception.DuplicateNameException;
 import ghidra.util.exception.InvalidInputException;
-import ghidra.util.task.*;
+import ghidra.util.task.TaskMonitor;
+import ghidra.util.task.UnknownProgressWrappingTaskMonitor;
 
 /**
  * Base class for symbols
@@ -43,10 +45,25 @@ public abstract class SymbolDB extends DatabaseObject implements Symbol {
 
 	private Record record;
 	private boolean isDeleting = false;
+	protected String name;
 	protected Address address;
 	protected SymbolManager symbolMgr;
 	protected Lock lock;
 
+	/**
+	 * Creates a Symbol that is just a placeholder for use when trying to find symbols by using
+	 * {@link Symbol#getID()}.   This is useful for locating symbols in Java collections when
+	 * a symbol has been deleted and the only remaining information is that symbol's ID.
+	 * 
+	 * @param manager the manager for the new symbol
+	 * @param address the address of the symbol
+	 * @param id the id of the symbol
+	 * @return the fake symbol
+	 */
+	static SymbolDB createSymbolPlaceholder(SymbolManager manager, Address address, long id) {
+		return new PlaceholderSymbolDB(manager, address, id);
+	}
+
 	SymbolDB(SymbolManager symbolMgr, DBObjectCache<SymbolDB> cache, Address address,
 			Record record) {
 		super(cache, record.getKey());
@@ -65,6 +82,11 @@ public abstract class SymbolDB extends DatabaseObject implements Symbol {
 
 	@Override
 	public String toString() {
+		// prefer cached name for speed; it may be stale; call getName() for current value
+		String temp = name;
+		if (temp != null) {
+			return temp;
+		}
 		return getName();
 	}
 
@@ -75,6 +97,7 @@ public abstract class SymbolDB extends DatabaseObject implements Symbol {
 
 	@Override
 	protected boolean refresh(Record rec) {
+		name = null;
 		if (record != null) {
 			if (rec == null) {
 				rec = symbolMgr.getSymbolRecord(key);
@@ -145,17 +168,29 @@ public abstract class SymbolDB extends DatabaseObject implements Symbol {
 		lock.acquire();
 		try {
 			checkIsValid();
-			if (record != null) {
-				return record.getString(SymbolDatabaseAdapter.SYMBOL_NAME_COL);
+			if (name == null) {
+				name = doGetName();
 			}
-
-			return SymbolUtilities.getDynamicName(symbolMgr.getProgram(), address);
+			return name;
 		}
 		finally {
 			lock.release();
 		}
 	}
 
+	/**
+	 * The code for creating the name content for this symbol.  This code will be called 
+	 * with the symbol's lock.
+	 * 
+	 * @return the name
+	 */
+	protected String doGetName() {
+		if (record != null) {
+			return record.getString(SymbolDatabaseAdapter.SYMBOL_NAME_COL);
+		}
+		return SymbolUtilities.getDynamicName(symbolMgr.getProgram(), address);
+	}
+
 	@Override
 	public Program getProgram() {
 		return symbolMgr.getProgram();
@@ -238,7 +273,7 @@ public abstract class SymbolDB extends DatabaseObject implements Symbol {
 		try {
 			checkIsValid();
 			if (monitor == null) {
-				monitor = TaskMonitorAdapter.DUMMY_MONITOR;
+				monitor = TaskMonitor.DUMMY;
 			}
 
 			if (monitor.getMaximum() == 0) {
@@ -275,7 +310,7 @@ public abstract class SymbolDB extends DatabaseObject implements Symbol {
 
 	@Override
 	public Reference[] getReferences() {
-		return getReferences(TaskMonitorAdapter.DUMMY_MONITOR);
+		return getReferences(TaskMonitor.DUMMY);
 	}
 
 	@Override
@@ -470,7 +505,11 @@ public abstract class SymbolDB extends DatabaseObject implements Symbol {
 
 	/**
 	 * Allow symbol implementations to validate the source when setting the name of
-	 * this symbol.
+	 * this symbol
+	 * 
+	 * @param newName the new name 
+	 * @param source the source type
+	 * @return the validated source type
 	 */
 	protected SourceType validateNameSource(String newName, SourceType source) {
 		return source;
@@ -482,6 +521,7 @@ public abstract class SymbolDB extends DatabaseObject implements Symbol {
 
 		lock.acquire();
 		try {
+			name = null;
 			checkDeleted();
 			checkEditOK();
 
@@ -535,6 +575,7 @@ public abstract class SymbolDB extends DatabaseObject implements Symbol {
 
 				record.setLongValue(SymbolDatabaseAdapter.SYMBOL_PARENT_COL, newNamespace.getID());
 				record.setString(SymbolDatabaseAdapter.SYMBOL_NAME_COL, newName);
+				name = newName;
 				updateSymbolSource(record, source);
 				updateRecord();
 
@@ -613,10 +654,16 @@ public abstract class SymbolDB extends DatabaseObject implements Symbol {
 		if (obj == this) {
 			return true;
 		}
+
 		Symbol s = (Symbol) obj;
+		if (getID() == s.getID()) {
+			return true;
+		}
+
 		if (!getName().equals(s.getName())) {
 			return false;
 		}
+
 		if (!getAddress().equals(s.getAddress())) {
 			return false;
 		}
@@ -776,6 +823,7 @@ public abstract class SymbolDB extends DatabaseObject implements Symbol {
 
 	/**
 	 * gets the generic symbol data 2 data.
+	 * @return the symbol data
 	 */
 	public int getSymbolData2() {
 		lock.acquire();
@@ -852,10 +900,64 @@ public abstract class SymbolDB extends DatabaseObject implements Symbol {
 
 	/**
 	 * Change the record and key associated with this symbol
-	 * @param the record.
+	 * @param record the record
 	 */
 	void setRecord(Record record) {
 		this.record = record;
 		keyChanged(record.getKey());
 	}
+
+	private static class PlaceholderSymbolDB extends SymbolDB {
+
+		PlaceholderSymbolDB(SymbolManager symbolMgr, Address address, long key) {
+			super(symbolMgr, null, address, key);
+		}
+
+		@Override
+		public boolean equals(Object obj) {
+			if ((obj == null) || (!(obj instanceof Symbol))) {
+				return false;
+			}
+			if (obj == this) {
+				return true;
+			}
+
+			// this class is only ever equal if the id matches
+			Symbol s = (Symbol) obj;
+			if (getID() == s.getID()) {
+				return true;
+			}
+			return false;
+		}
+
+		@Override
+		public SymbolType getSymbolType() {
+			throw new IllegalArgumentException();
+		}
+
+		@Override
+		public ProgramLocation getProgramLocation() {
+			throw new IllegalArgumentException();
+		}
+
+		@Override
+		public boolean isExternal() {
+			throw new IllegalArgumentException();
+		}
+
+		@Override
+		public Object getObject() {
+			throw new IllegalArgumentException();
+		}
+
+		@Override
+		public boolean isPrimary() {
+			throw new IllegalArgumentException();
+		}
+
+		@Override
+		public boolean isValidParent(Namespace parent) {
+			throw new IllegalArgumentException();
+		}
+	}
 }

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/symbol/SymbolManager.java

@@ -2274,6 +2274,11 @@ public class SymbolManager implements SymbolTable, ManagerDB {
 		return new NamespaceDB(s, namespaceMgr);
 	}
 
+	@Override
+	public Symbol createSymbolPlaceholder(Address address, long id) {
+		return SymbolDB.createSymbolPlaceholder(this, address, id);
+	}
+
 	/**
 	 * Creates a symbol, specifying all information for the record.  This method is not on the
 	 * public interface and is only intended for program API internal use.  The user of this
@@ -2675,5 +2680,4 @@ class SymbolMatcher implements Predicate<Symbol> {
 		SymbolType type = s.getSymbolType();
 		return type == type1;
 	}
-
 }

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/database/symbol/VariableSymbolDB.java

@@ -203,7 +203,7 @@ public class VariableSymbolDB extends SymbolDB {
 	}
 
 	@Override
-	public String getName() {
+	protected String doGetName() {
 		if (!checkIsValid()) {
 			// TODO: SCR
 			return "[Invalid VariableSymbol - Deleted!]";
@@ -213,7 +213,7 @@ public class VariableSymbolDB extends SymbolDB {
 			if (getSource() == SourceType.DEFAULT) {
 				return getParamName();
 			}
-			String storedName = super.getName();
+			String storedName = super.doGetName();
 			if (SymbolUtilities.isDefaultParameterName(storedName)) {
 				return getParamName();
 			}
@@ -232,7 +232,7 @@ public class VariableSymbolDB extends SymbolDB {
 		// TODO: we use to check for a default name and regenerate new default name but we should
 		// not need to do this if source remains at default
 
-		return super.getName();
+		return super.doGetName();
 	}
 
 	@Override

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/symbol/SymbolTable.java

@@ -543,4 +543,15 @@ public interface SymbolTable {
 	public Namespace createNameSpace(Namespace parent, String name, SourceType source)
 			throws DuplicateNameException, InvalidInputException;
 
+	/**
+	 * Creates a Symbol that is just a placeholder for use when trying to find symbols by using
+	 * {@link Symbol#getID()}.   This is useful for locating symbols in Java collections when
+	 * a symbol has been deleted and the only remaining information is that symbol's ID.
+	 * 
+	 * @param address the address of the symbol
+	 * @param id the id of the symbol
+	 * @return the fake symbol
+	 */
+	public Symbol createSymbolPlaceholder(Address address, long id);
+
 }

Ghidra/Framework/SoftwareModeling/src/main/java/ghidra/program/model/symbol/SymbolUtilities.java

@@ -897,7 +897,11 @@ public class SymbolUtilities {
 			if (symbol.isExternal()) {
 				return "External Function";
 			}
+
 			Function func = (Function) symbol.getObject();
+			if (func == null) {
+				return null; // symbol deleted
+			}
 			if (func.isThunk()) {
 				return "Thunk Function";
 			}