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

removed odd comparator name/structure; update ProgramSymbol context to
use Symbols and not IDs

Ghidra/Features/Base/src/main/java/ghidra/app/context/ProgramSymbolActionContext.java

@@ -16,80 +16,34 @@
 package ghidra.app.context;
 
 import java.awt.Component;
-import java.util.Iterator;
-import java.util.NoSuchElementException;
+import java.util.*;
 
 import docking.ComponentProvider;
 import ghidra.program.model.listing.Program;
 import ghidra.program.model.symbol.Symbol;
-import ghidra.program.model.symbol.SymbolIterator;
 
 public class ProgramSymbolActionContext extends ProgramActionContext {
 
-	private final long[] symbolIDs;
+	private List<Symbol> symbols = new ArrayList<Symbol>();
 
-	public ProgramSymbolActionContext(ComponentProvider provider, Program program, long[] symbolIDs,
-			Component sourceComponent) {
+	public ProgramSymbolActionContext(ComponentProvider provider, Program program,
+			List<Symbol> symbols, Component sourceComponent) {
 		super(provider, program, sourceComponent);
-		this.symbolIDs = symbolIDs;
+		this.symbols = symbols == null ? Collections.emptyList() : symbols;
 	}
 
 	public int getSymbolCount() {
-		return symbolIDs != null ? symbolIDs.length : 0;
+		return symbols.size();
 	}
 
 	public Symbol getFirstSymbol() {
-		if (symbolIDs == null || symbolIDs.length == 0) {
+		if (symbols.isEmpty()) {
 			return null;
 		}
-		return program.getSymbolTable().getSymbol(symbolIDs[0]);
+		return symbols.get(0);
 	}
 
-	public SymbolIterator getSymbols() {
-		return new MySymbolIterator();
-	}
-
-	private class MySymbolIterator implements SymbolIterator {
-
-		private int index = -1;
-		private Symbol symbol = null;
-
-		@Override
-		public void remove() {
-			throw new UnsupportedOperationException();
-		}
-
-		@Override
-		public Iterator<Symbol> iterator() {
-			return this;
-		}
-
-		@Override
-		public boolean hasNext() {
-			if (symbol != null) {
-				return true;
-			}
-			if (symbolIDs == null) {
-				return false;
-			}
-			while (index < (symbolIDs.length - 1)) {
-				symbol = program.getSymbolTable().getSymbol(symbolIDs[++index]);
-				if (symbol != null) {
-					return true;
-				}
-			}
-			return false;
-		}
-
-		@Override
-		public Symbol next() {
-			if (hasNext()) {
-				Symbol s = symbol;
-				symbol = null;
-				return s;
-			}
-			throw new NoSuchElementException();
-		}
-
+	public Iterable<Symbol> getSymbols() {
+		return symbols;
 	}
 }

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/functionwindow/FunctionRowObject.java

@@ -20,11 +20,9 @@ import ghidra.program.model.listing.Function;
 class FunctionRowObject implements Comparable<FunctionRowObject> {
 
 	private final Function function;
-	private final long key;
 
 	FunctionRowObject(Function function) {
 		this.function = function;
-		this.key = function.getID();
 	}
 
 	Function getFunction() {
@@ -33,10 +31,7 @@ class FunctionRowObject implements Comparable<FunctionRowObject> {
 
 	@Override
 	public int hashCode() {
-		final int prime = 31;
-		int result = 1;
-		result = prime * result + (int) (key ^ (key >>> 32));
-		return result;
+		return (int) function.getID();
 	}
 
 	@Override
@@ -50,19 +45,21 @@ class FunctionRowObject implements Comparable<FunctionRowObject> {
 		if (getClass() != obj.getClass()) {
 			return false;
 		}
+
+		long key = function.getID();
 		FunctionRowObject other = (FunctionRowObject) obj;
-		if (key != other.key) {
+		if (key != other.function.getID()) {
 			return false;
 		}
 		return true;
 	}
 
 	long getKey() {
-		return key;
+		return function.getID();
 	}
 
 	@Override
 	public int compareTo(FunctionRowObject o) {
-		return ((Long) key).compareTo(o.key);
+		return ((Long) function.getID()).compareTo(o.function.getID());
 	}
 }

Ghidra/Features/Base/src/main/java/ghidra/app/plugin/core/symboltree/SymbolTreeActionContext.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.
@@ -16,11 +15,15 @@
  */
 package ghidra.app.plugin.core.symboltree;
 
+import java.util.ArrayList;
+import java.util.List;
+
+import javax.swing.tree.TreePath;
+
 import ghidra.app.context.ProgramSymbolActionContext;
 import ghidra.app.plugin.core.symboltree.nodes.SymbolNode;
 import ghidra.program.model.listing.Program;
-
-import javax.swing.tree.TreePath;
+import ghidra.program.model.symbol.Symbol;
 
 public class SymbolTreeActionContext extends ProgramSymbolActionContext {
 
@@ -28,7 +31,7 @@ public class SymbolTreeActionContext extends ProgramSymbolActionContext {
 
 	SymbolTreeActionContext(SymbolTreeProvider provider, Program program, SymbolGTree tree,
 			TreePath[] selectionPaths) {
-		super(provider, program, getSymbolIDs(selectionPaths), tree);
+		super(provider, program, getSymbols(selectionPaths), tree);
 		this.selectionPaths = selectionPaths;
 	}
 
@@ -51,23 +54,23 @@ public class SymbolTreeActionContext extends ProgramSymbolActionContext {
 		return null;
 	}
 
-	private static long[] getSymbolIDs(TreePath[] selectionPaths) {
+	private static List<Symbol> getSymbols(TreePath[] selectionPaths) {
 		if (selectionPaths == null) {
 			return null;
 		}
-		long[] symbolIDs = new long[selectionPaths.length];
-		int index = 0;
+
+		List<Symbol> symbols = new ArrayList<>();
 		for (TreePath treePath : selectionPaths) {
 			Object object = treePath.getLastPathComponent();
 			if (object instanceof SymbolNode) {
 				SymbolNode symbolNode = (SymbolNode) object;
-				symbolIDs[index++] = symbolNode.getSymbolID();
+				symbols.add(symbolNode.getSymbol());
 			}
 			else {
 				// Do not return symbols if selection contains non-symbolNodes
 				return null;
 			}
 		}
-		return symbolIDs;
+		return symbols;
 	}
 }

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

@@ -185,7 +185,7 @@ class SymbolPanel extends JPanel {
 		return symTable.getRowCount();
 	}
 
-	List<Symbol> getSelectedSymbolKeys() {
+	List<Symbol> getSelectedSymbols() {
 		int[] rows = symTable.getSelectedRows();
 		return tableModel.getRowObjects(rows);
 	}

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

@@ -75,17 +75,12 @@ class SymbolProvider extends ComponentProviderAdapter {
 			return null;
 		}
 
-		List<Symbol> rowObjects = symbolPanel.getSelectedSymbolKeys();
-		long[] symbolIDs = new long[rowObjects.size()];
-		int index = 0;
-		for (Symbol obj : rowObjects) {
-			symbolIDs[index++] = obj.getID();
-		}
-		return new ProgramSymbolActionContext(this, program, symbolIDs, getTable());
+		List<Symbol> symbols = symbolPanel.getSelectedSymbols();
+		return new ProgramSymbolActionContext(this, program, symbols, getTable());
 	}
 
 	void deleteSymbols() {
-		List<Symbol> rowObjects = symbolPanel.getSelectedSymbolKeys();
+		List<Symbol> rowObjects = symbolPanel.getSelectedSymbols();
 		symbolKeyModel.delete(rowObjects);
 	}
 
@@ -94,7 +89,7 @@ class SymbolProvider extends ComponentProviderAdapter {
 	}
 
 	Symbol getCurrentSymbol() {
-		List<Symbol> rowObjects = symbolPanel.getSelectedSymbolKeys();
+		List<Symbol> rowObjects = symbolPanel.getSelectedSymbols();
 		if (rowObjects != null && rowObjects.size() >= 1) {
 			return rowObjects.get(0);
 		}

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

@@ -20,7 +20,8 @@ import java.util.*;
 import javax.swing.event.TableModelEvent;
 import javax.swing.table.TableModel;
 
-import docking.widgets.table.sort.*;
+import docking.widgets.table.sort.DefaultColumnComparator;
+import docking.widgets.table.sort.RowBasedColumnComparator;
 import ghidra.util.Swing;
 import ghidra.util.datastruct.WeakDataStructureFactory;
 import ghidra.util.datastruct.WeakSet;
@@ -329,7 +330,7 @@ public abstract class AbstractSortedTableModel<T> extends AbstractGTableModel<T>
 	 * @return the comparator 
 	 */
 	protected Comparator<T> createSortComparator(int columnIndex) {
-		return new RowToColumnComparator<>(this, columnIndex, new DefaultColumnComparator(),
+		return new RowBasedColumnComparator<>(this, columnIndex, new DefaultColumnComparator(),
 			new StringBasedBackupRowToColumnComparator());
 	}
 
@@ -469,11 +470,11 @@ public abstract class AbstractSortedTableModel<T> extends AbstractGTableModel<T>
 		}
 	}
 
-	private class StringBasedBackupRowToColumnComparator implements BackupColumnComparator<T> {
+	private class StringBasedBackupRowToColumnComparator implements Comparator<Object> {
 
 		@Override
-		public int compare(T t1, T t2, Object c1, Object c2) {
-			if (t1 == t2) {
+		public int compare(Object c1, Object c2) {
+			if (c1 == c2) {
 				return 0;
 			}
 

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

@@ -178,11 +178,11 @@ public abstract class GDynamicColumnTableModel<ROW_TYPE, DATA_SOURCE>
 		Comparator<Object> columnComparator = createSortComparatorForColumn(columnIndex);
 		if (columnComparator != null) {
 			// the given column has its own comparator; wrap and us that
-			return new RowToColumnComparator<>(this, columnIndex, columnComparator);
+			return new RowBasedColumnComparator<>(this, columnIndex, columnComparator);
 		}
 
-		return new RowToColumnComparator<>(this, columnIndex, new DefaultColumnComparator(),
-			new ColumnRenderedValueBackupRowComparator<>(this, columnIndex));
+		return new RowBasedColumnComparator<>(this, columnIndex, new DefaultColumnComparator(),
+			new ColumnRenderedValueBackupComparator<>(this, columnIndex));
 	}
 
 	/**

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

@@ -1,50 +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 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/ColumnRenderedValueBackupComparator.java

@@ -15,6 +15,8 @@
  */
 package docking.widgets.table.sort;
 
+import java.util.Comparator;
+
 import docking.widgets.table.*;
 import ghidra.docking.settings.Settings;
 import ghidra.util.table.column.GColumnRenderer;
@@ -27,7 +29,7 @@ import ghidra.util.table.column.GColumnRenderer.ColumnConstraintFilterMode;
  * 
  * @param <T> the row type 
  */
-public class ColumnRenderedValueBackupRowComparator<T> implements BackupColumnComparator<T> {
+public class ColumnRenderedValueBackupComparator<T> implements Comparator<Object> {
 
 	protected int sortColumn;
 	protected DynamicColumnTableModel<T> model;
@@ -36,7 +38,7 @@ public class ColumnRenderedValueBackupRowComparator<T> implements BackupColumnCo
 	// for column filtering only
 	private boolean supportsColumnSorting = true;
 
-	public ColumnRenderedValueBackupRowComparator(DynamicColumnTableModel<T> model,
+	public ColumnRenderedValueBackupComparator(DynamicColumnTableModel<T> model,
 			int sortColumn) {
 		this.model = model;
 		this.sortColumn = sortColumn;
@@ -54,8 +56,8 @@ public class ColumnRenderedValueBackupRowComparator<T> implements BackupColumnCo
 	}
 
 	@Override
-	public int compare(T t1, T t2, Object c1, Object c2) {
-		if (t1 == t2) {
+	public int compare(Object c1, Object c2) {
+		if (c1 == c2) {
 			return 0;
 		}
 

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

@@ -27,13 +27,12 @@ import docking.widgets.table.TableComparators;
  * 
  * @param <T> the row type
  */
-public class RowToColumnComparator<T> implements Comparator<T> {
+public class RowBasedColumnComparator<T> implements Comparator<T> {
 
 	protected RowObjectTableModel<T> model;
 	protected int sortColumn;
 	protected Comparator<Object> columnComparator;
-	protected BackupColumnComparator<T> backupRowComparator =
-		BackupColumnComparator.getNoSortComparator();
+	protected Comparator<Object> backupRowComparator = TableComparators.getNoSortComparator();
 
 	/**
 	 * Constructs this class with the given column comparator that will get called after the
@@ -43,7 +42,7 @@ public class RowToColumnComparator<T> implements Comparator<T> {
 	 * @param sortColumn the column being sorted
 	 * @param comparator the column comparator to use for sorting
 	 */
-	public RowToColumnComparator(RowObjectTableModel<T> model, int sortColumn,
+	public RowBasedColumnComparator(RowObjectTableModel<T> model, int sortColumn,
 			Comparator<Object> comparator) {
 		this.model = model;
 		this.sortColumn = sortColumn;
@@ -60,8 +59,8 @@ public class RowToColumnComparator<T> implements Comparator<T> {
 	 * @param comparator the column comparator to use for sorting
 	 * @param backupRowComparator the backup row comparator
 	 */
-	public RowToColumnComparator(RowObjectTableModel<T> model, int sortColumn,
-			Comparator<Object> comparator, BackupColumnComparator<T> backupRowComparator) {
+	public RowBasedColumnComparator(RowObjectTableModel<T> model, int sortColumn,
+			Comparator<Object> comparator, Comparator<Object> backupRowComparator) {
 		this.model = model;
 		this.sortColumn = sortColumn;
 		this.columnComparator = Objects.requireNonNull(comparator);
@@ -98,7 +97,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, value1, value2);
+		return backupRowComparator.compare(value1, value2);
 	}
 
 	protected Object getColumnValue(T t) {

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

@@ -15,16 +15,16 @@
  */
 package docking.widgets.table.threaded;
 
-import docking.widgets.table.sort.ColumnRenderedValueBackupRowComparator;
-import docking.widgets.table.sort.RowToColumnComparator;
+import docking.widgets.table.sort.ColumnRenderedValueBackupComparator;
+import docking.widgets.table.sort.RowBasedColumnComparator;
 
 /**
- * A version of {@link ColumnRenderedValueBackupRowComparator} that uses the 
+ * A version of {@link ColumnRenderedValueBackupComparator} that uses the 
  * {@link ThreadedTableModel}'s cache for column lookups
  *
  * @param <T> the row type
  */
-public class ThreadedBackupRowComparator<T> extends ColumnRenderedValueBackupRowComparator<T> {
+public class ThreadedBackupRowComparator<T> extends ColumnRenderedValueBackupComparator<T> {
 
 	private ThreadedTableModel<T, ?> threadedModel;
 
@@ -34,7 +34,7 @@ public class ThreadedBackupRowComparator<T> extends ColumnRenderedValueBackupRow
 	 * 
 	 * @param model the table model using this comparator
 	 * @param sortColumn the column being sorted
-	 * @see RowToColumnComparator
+	 * @see RowBasedColumnComparator
 	 */
 	public ThreadedBackupRowComparator(ThreadedTableModel<T, ?> model, int sortColumn) {
 		super(model, sortColumn);

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

@@ -17,8 +17,7 @@ package docking.widgets.table.threaded;
 
 import java.util.Comparator;
 
-import docking.widgets.table.sort.BackupColumnComparator;
-import docking.widgets.table.sort.RowToColumnComparator;
+import docking.widgets.table.sort.RowBasedColumnComparator;
 
 /**
  * A comparator for comparing table column values for threaded table models.  This comparator
@@ -26,7 +25,7 @@ import docking.widgets.table.sort.RowToColumnComparator;
  *
  * @param <T> the row type
  */
-public class ThreadedTableColumnComparator<T> extends RowToColumnComparator<T> {
+public class ThreadedTableColumnComparator<T> extends RowBasedColumnComparator<T> {
 	private ThreadedTableModel<T, ?> threadedModel;
 
 	/**
@@ -36,7 +35,7 @@ public class ThreadedTableColumnComparator<T> extends RowToColumnComparator<T> {
 	 * @param model the table model using this comparator
 	 * @param sortColumn the column being sorted
 	 * @param comparator the column comparator to use for sorting
-	 * @see RowToColumnComparator
+	 * @see RowBasedColumnComparator
 	 */
 	public ThreadedTableColumnComparator(ThreadedTableModel<T, ?> model, int sortColumn,
 			Comparator<Object> comparator) {
@@ -53,10 +52,10 @@ public class ThreadedTableColumnComparator<T> extends RowToColumnComparator<T> {
 	 * @param sortColumn the column being sorted
 	 * @param comparator the column comparator to use for sorting
 	 * @param backupRowComparator the backup row comparator
-	 * @see RowToColumnComparator
+	 * @see RowBasedColumnComparator
 	 */
 	public ThreadedTableColumnComparator(ThreadedTableModel<T, ?> model, int sortColumn,
-			Comparator<Object> comparator, BackupColumnComparator<T> backupRowComparator) {
+			Comparator<Object> comparator, Comparator<Object> backupRowComparator) {
 		super(model, sortColumn, comparator, backupRowComparator);
 		this.threadedModel = model;
 	}