GP-5654 - Data Type Manager - Speed improvements for deleting data types

This commit is contained in:
dragonmacher 2025-05-13 14:27:17 -04:00
parent 0ebc4c9608
commit 404191cdaa
41 changed files with 381 additions and 233 deletions

View file

@ -314,7 +314,7 @@ public class ProjectDataTypeManager extends StandAloneDataTypeManager
}
@Override
public void close() {
public synchronized void close() {
// do nothing - cannot close a project data type manager
// dispose should be invoked by the owner of the instance
}

View file

@ -2707,20 +2707,18 @@ public class CodeManager implements ErrorHandler, ManagerDB {
public void clearData(Set<Long> dataTypeIDs, TaskMonitor monitor) throws CancelledException {
lock.acquire();
try {
List<Address> addrs = new ArrayList<>();
List<Address> toClear = new ArrayList<>();
RecordIterator it = dataAdapter.getRecords();
while (it.hasNext()) {
monitor.checkCancelled();
DBRecord rec = it.next();
long id = rec.getLongValue(DataDBAdapter.DATA_TYPE_ID_COL);
for (long dataTypeID : dataTypeIDs) {
if (id == dataTypeID) {
addrs.add(addrMap.decodeAddress(rec.getKey()));
break;
}
if (dataTypeIDs.contains(id)) {
toClear.add(addrMap.decodeAddress(rec.getKey()));
}
}
for (Address addr : addrs) {
for (Address addr : toClear) {
monitor.checkCancelled();
clearCodeUnits(addr, addr, false, monitor);
}

View file

@ -4,9 +4,9 @@
* 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.
@ -27,8 +27,7 @@ import ghidra.program.model.data.*;
import ghidra.program.model.data.DataTypeConflictHandler.ConflictResult;
import ghidra.util.InvalidNameException;
import ghidra.util.Lock;
import ghidra.util.exception.AssertException;
import ghidra.util.exception.DuplicateNameException;
import ghidra.util.exception.*;
import ghidra.util.task.TaskMonitor;
/**
@ -308,9 +307,6 @@ class CategoryDB extends DatabaseObject implements Category {
return null;
}
/**
* @see ghidra.program.model.data.Category#removeCategory(java.lang.String, ghidra.util.task.TaskMonitor)
*/
@Override
public boolean removeCategory(String categoryName, TaskMonitor monitor) {
mgr.lock.acquire();
@ -320,6 +316,7 @@ class CategoryDB extends DatabaseObject implements Category {
if (c == null) {
return false;
}
Category[] cats = c.getCategories();
for (Category cat : cats) {
if (monitor.isCancelled()) {
@ -327,13 +324,16 @@ class CategoryDB extends DatabaseObject implements Category {
}
c.removeCategory(cat.getName(), monitor);
}
DataType[] dts = c.getDataTypes();
for (DataType dt : dts) {
if (monitor.isCancelled()) {
return false;
}
mgr.remove(dt, monitor);
List<DataType> dtList = Arrays.asList(dts);
try {
mgr.remove(dtList, monitor);
}
catch (CancelledException e) {
return false;
}
try {
mgr.getCategoryDBAdapter().removeCategory(c.getKey());
subcategoryMap.remove(categoryName);
@ -565,7 +565,7 @@ class CategoryDB extends DatabaseObject implements Category {
throw new IllegalArgumentException(
"can't remove dataType from category that its not a member of!");
}
return mgr.remove(type, monitor);
return mgr.remove(type);
}
/**

View file

@ -164,7 +164,7 @@ abstract public class DataTypeManagerDB implements DataTypeManager {
*/
private Set<Long> blockedRemovalsByID;
// TODO: idsToDataTypeMap may have issue since there could be a one to many mapping
// Note: idsToDataTypeMap may have issue since there could be a one to many mapping
// (e.g., type with same UniversalID could be in multiple categories unless specifically
// prevented during resolve)
private IdsToDataTypeMap idsToDataTypeMap = new IdsToDataTypeMap();
@ -172,7 +172,7 @@ abstract public class DataTypeManagerDB implements DataTypeManager {
private ThreadLocal<EquivalenceCache> equivalenceCache = new ThreadLocal<>();
private IdentityHashMap<DataType, DataType> resolveCache;
private TreeSet<ResolvePair> resolveQueue; // TODO: is TreeSet really needed?
private TreeSet<ResolvePair> resolveQueue; // Note: is TreeSet really needed?
private LinkedList<DataType> conflictQueue = new LinkedList<>();
private boolean isBulkRemoving;
@ -1421,14 +1421,14 @@ abstract public class DataTypeManagerDB implements DataTypeManager {
if (!(dataType instanceof Enum)) {
return false;
}
// TODO: implement doReplaceWith
// DT: implement doReplaceWith
existingDataType.replaceWith(dataType);
}
else if (existingDataType instanceof TypedefDB) {
if (!(dataType instanceof TypeDef)) {
return false;
}
// TODO: implement doReplaceWith
// DT: implement doReplaceWith
existingDataType.replaceWith(dataType);
}
else {
@ -1733,7 +1733,6 @@ abstract public class DataTypeManagerDB implements DataTypeManager {
}
catch (DataTypeDependencyException e) {
// new type refers to old type - fallthrough to RENAME_AND_ADD
// TODO: alternatively we could throw an exception
}
case RENAME_AND_ADD: // default handler behavior
@ -1760,7 +1759,6 @@ abstract public class DataTypeManagerDB implements DataTypeManager {
private void replaceEquivalentLocalWithSourceDataType(DataType dataType,
SourceArchive sourceArchive, DataType existingDataType) {
// Since it's equivalent, set its source, ID, and replace its components.
// TODO: Need a better way to do this.
existingDataType.setSourceArchive(sourceArchive);
((DataTypeDB) existingDataType).setUniversalID(dataType.getUniversalID());
existingDataType.replaceWith(dataType);
@ -1786,9 +1784,9 @@ abstract public class DataTypeManagerDB implements DataTypeManager {
lock.acquire();
boolean isEquivalenceCacheOwner = activateEquivalenceCache();
boolean isResolveCacheOwner = activateResolveCache();
// TODO: extended hold time on lock may cause the GUI to become
// unresponsive. Consider releasing lock between resolves, although
// this exposes risk of having active resolve queue/cache without lock
// Note: extended hold time on lock may cause the GUI to become unresponsive. Consider
// releasing lock between resolves, although this exposes risk of having active resolve
// queue/cache without lock.
try {
monitor.setMessage("Adding datatypes...");
monitor.setMaximum(dataTypes.size());
@ -1879,7 +1877,7 @@ abstract public class DataTypeManagerDB implements DataTypeManager {
@Override
public DataType replaceDataType(DataType existingDt, DataType replacementDt,
boolean updateCategoryPath) throws DataTypeDependencyException {
// TODO: we should probably disallow replacementDt to be an instanceof
// Note: we should probably disallow replacementDt to be an instanceof
// Dynamic or FactoryDataType
lock.acquire();
try {
@ -2342,11 +2340,30 @@ abstract public class DataTypeManagerDB implements DataTypeManager {
if (id <= 0) { // removal of certain special types not permitted
return false;
}
idsToDelete.add(id);
removeQueuedDataTypes();
return true;
}
private void removeInternal(List<DataType> dataTypes) {
for (DataType dt : dataTypes) {
if (!contains(dt)) {
continue;
}
long id = getID(dt);
if (id <= 0) { // removal of certain special types not permitted
continue;
}
idsToDelete.add(id);
}
removeQueuedDataTypes();
}
private void removeQueuedDataTypes() {
// collect all datatype to be removed and notify children which may also get queued
@ -2358,7 +2375,7 @@ abstract public class DataTypeManagerDB implements DataTypeManager {
deletedIds.add(id);
}
// perform any neccessary external use removals
// perform any necessary external use removals
deleteDataTypesUsed(deletedIds);
// perform actual database updates (e.g., record removal, change notifications, etc.)
@ -2375,7 +2392,7 @@ abstract public class DataTypeManagerDB implements DataTypeManager {
}
/**
* Allow extensions to perform any neccessary fixups for all datatype removals listed.
* Allow extensions to perform any necessary fixups for all datatype removals listed.
* @param deletedIds list of IDs for all datatypes which are getting removed.
*/
protected abstract void deleteDataTypesUsed(Set<Long> deletedIds);
@ -2399,7 +2416,7 @@ abstract public class DataTypeManagerDB implements DataTypeManager {
}
@Override
public boolean remove(DataType dataType, TaskMonitor monitor) {
public boolean remove(DataType dataType) {
lock.acquire();
try {
return removeInternal(dataType);
@ -2409,6 +2426,32 @@ abstract public class DataTypeManagerDB implements DataTypeManager {
}
}
@Override
public void remove(List<DataType> dataTypes, TaskMonitor monitor) throws CancelledException {
lock.acquire();
try {
// perform the delete in chunks so an excessively large list will still be cancellable
int n = dataTypes.size();
int chunk = 1000;
int chunkEnd = n < chunk ? n : chunk;
int start = 0;
while (start < chunkEnd) {
monitor.checkCancelled();
List<DataType> subList = dataTypes.subList(start, chunkEnd);
removeInternal(subList);
start = chunkEnd;
int nextChunk = chunkEnd + chunk;
chunkEnd = n < nextChunk ? n : nextChunk;
}
}
finally {
lock.release();
}
}
@Override
public void associateDataTypeWithArchive(DataType datatype, SourceArchive archive) {
if (!contains(datatype)) {
@ -2846,7 +2889,7 @@ abstract public class DataTypeManagerDB implements DataTypeManager {
CategoryPath catPath = catDB.getCategoryPath();
String classPath = record.getString(BuiltinDBAdapter.BUILT_IN_CLASSNAME_COL);
String name = record.getString(BuiltinDBAdapter.BUILT_IN_NAME_COL);
try { // TODO: !! Can we look for alternate constructor which takes DTM argument
try {
Class<?> c;
try {
c = Class.forName(classPath);
@ -4110,7 +4153,7 @@ abstract public class DataTypeManagerDB implements DataTypeManager {
try {
// Initialization of dataOrganization may never have been established
// if either an architecture has never been specified or a language
// error occured during initializtion. In such cases the stored
// error occurred during initialization. In such cases the stored
// data organization should be used if available.
dataOrganization = readDataOrganization();
}
@ -4212,7 +4255,7 @@ abstract public class DataTypeManagerDB implements DataTypeManager {
private TreeSet<String> buildDefinedCallingConventionSet() {
// Include all calling conventions defined by associated architecure compiler spec
// Include all calling conventions defined by associated architecture compiler spec
TreeSet<String> nameSet = new TreeSet<>();
ProgramArchitecture arch = getProgramArchitecture();
if (arch != null) {
@ -4536,8 +4579,7 @@ abstract public class DataTypeManagerDB implements DataTypeManager {
}
}
private record DedupedConflicts(int processCnt, int replaceCnt) {
}
private record DedupedConflicts(int processCnt, int replaceCnt) {}
private DedupedConflicts doDedupeConflicts(DataType dataType) {
@ -4761,7 +4803,6 @@ abstract public class DataTypeManagerDB implements DataTypeManager {
resolvedDt.postPointerResolve(resolvePair.definitionDt, handler);
}
}
// TODO: catch exceptions if needed
finally {
resolvedDt.resolving = false;
resolvedDt.pointerPostResolveRequired = false;

View file

@ -4,9 +4,9 @@
* 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.
@ -435,7 +435,7 @@ public abstract class ProgramBasedDataTypeManagerDB extends DataTypeManagerDB
Address addr = data.getAddress();
for (Data parent = data.getParent(); parent != null; parent = parent.getParent()) {
DataType dataType = parent.getDataType();
if (!(dataType instanceof Array a)) {
if (!(dataType instanceof Array)) {
break;
}
addr = parent.getAddress();

View file

@ -266,6 +266,10 @@ public class ProgramDataTypeManager extends ProgramBasedDataTypeManagerDB implem
// use could be an issue.
try {
// TODO: Should use replacement type instead of clearing
// Note: use of DUMMY here is intentional, since we do not want to interrupt the
// deleting of these types, as they may have a relationship that we wish to preserve.
// All need to be deleted to remain in a consistent state.
program.getCodeManager().clearData(deletedIds, TaskMonitor.DUMMY);
}
catch (CancelledException e) {

View file

@ -210,7 +210,7 @@ public final class BuiltInDataTypeManager extends StandAloneDataTypeManager {
}
@Override
public boolean remove(DataType dataType, TaskMonitor monitor) {
public boolean remove(DataType dataType) {
throw new UnsupportedOperationException();
}

View file

@ -75,7 +75,7 @@ public interface DataTypeManager {
/**
* Get the program architecture information which has been associated with this
* datatype manager. If {@link #getProgramArchitecture()} returns null this method
* data type manager. If {@link #getProgramArchitecture()} returns null this method
* may still return information if the program architecture was set on an archive but unable
* to properly instantiate.
* @return program architecture summary if it has been set
@ -83,16 +83,16 @@ public interface DataTypeManager {
public String getProgramArchitectureSummary();
/**
* Returns true if the given category path exists in this datatype manager
* Returns true if the given category path exists in this data type manager
* @param path the path
* @return true if the given category path exists in this datatype manager
* @return true if the given category path exists in this data type manager
*/
public boolean containsCategory(CategoryPath path);
/**
* Returns a unique name not currently used by any other dataType or category
* Returns a unique name not currently used by any other data type or category
* with the same baseName. This does not produce a conflict name and is intended
* to be used when generating an artifical datatype name only (e.g., {@code temp_1},
* to be used when generating an artificial data type name only (e.g., {@code temp_1},
* {@code temp_2}; for {@code baseName="temp"}.
*
* @param path the path of the name
@ -102,12 +102,11 @@ public interface DataTypeManager {
public String getUniqueName(CategoryPath path, String baseName);
/**
* Returns a dataType that is "in" (ie suitable implementation) this
* Manager, creating a new one if necessary. Also the returned dataType
* will be in a category in this dataTypeManager that is equivalent to the
* category of the passed in dataType.
* Returns a data type that is "in" this Manager, creating a new one if necessary. Also the
* returned data type will be in a category in this manager that is equivalent to the
* category of the passed in data type.
* @param dataType the dataType to be resolved.
* @param handler used to resolve conflicts with existing dataTypes.
* @param handler used to resolve conflicts with existing data types.
* @return an equivalent dataType that "belongs" to this dataTypeManager.
*/
public DataType resolve(DataType dataType, DataTypeConflictHandler handler);
@ -309,22 +308,48 @@ public interface DataTypeManager {
public void removeInvalidatedListener(InvalidatedListener listener);
/**
* Remove the given datatype from this manager.
* Remove the given data type from this manager.
* <br>
* NOTE: Any use of the specified datatype within a {@link FunctionDefinition} will be
* converted to the {@link DataType#DEFAULT default 'undefined' datatype}. Any use within
* a {@link Structure} or {@link Union} will be converted to the {@link BadDataType} as
* a placeholder to retain the component's field name and length (the comment will be prefixed
* with a message indicating the remval of the old datatype.
* with a message indicating the removal of the old datatype.
*
* @param dataType the dataType to be removed
* @param monitor the task monitor
* @param dataType the data type to be removed
* @return true if the data type existed and was removed
*/
public boolean remove(DataType dataType, TaskMonitor monitor);
public boolean remove(DataType dataType);
/**
* Return true if the given dataType exists in this data type manager
* Deprecated. Use {@link #remove(DataType)}.
* @param dataType the data type
* @param monitor the monitor
* @return true if the data type existed and was removed
* @deprecated use {@link #remove(DataType)}
*/
@Deprecated(since = "10.4", forRemoval = true)
public default boolean remove(DataType dataType, TaskMonitor monitor) {
return remove(dataType);
}
/**
* Remove the given data types from this manager.
* <br>
* NOTE: Any use of the specified data types within a {@link FunctionDefinition} will be
* converted to the {@link DataType#DEFAULT default 'undefined' datatype}. Any use within
* a {@link Structure} or {@link Union} will be converted to the {@link BadDataType} as
* a placeholder to retain the component's field name and length (the comment will be prefixed
* with a message indicating the removal of the old datatype.
*
* @param dataTypes the data types to be removed
* @param monitor the monitor
* @throws CancelledException if the user cancels
*/
public void remove(List<DataType> dataTypes, TaskMonitor monitor) throws CancelledException;
/**
* Return true if the given data type exists in this data type manager
*
* @param dataType the type
* @return true if the type is in this manager
@ -727,7 +752,7 @@ public interface DataTypeManager {
public Collection<String> getDefinedCallingConventionNames();
/**
* Get the default calling convention's prototype model in this datatype manager if known.
* Get the default calling convention's prototype model in this data type manager if known.
*
* @return the default calling convention prototype model or null.
*/

View file

@ -26,7 +26,6 @@ import ghidra.program.model.symbol.*;
import ghidra.util.InvalidNameException;
import ghidra.util.exception.DuplicateNameException;
import ghidra.util.exception.InvalidInputException;
import ghidra.util.task.TaskMonitor;
public class DataTypeSymbol {
private Symbol sym; // Traditional symbol object
@ -151,7 +150,7 @@ public class DataTypeSymbol {
}
// remove unused override signature
program.getDataTypeManager().remove(getDataType(), TaskMonitor.DUMMY);
program.getDataTypeManager().remove(getDataType());
}
public static DataTypeSymbol readSymbol(String cat, Symbol s) {

View file

@ -27,8 +27,6 @@ import generic.test.AbstractGenericTest;
import ghidra.program.model.data.*;
import ghidra.util.InvalidNameException;
import ghidra.util.exception.DuplicateNameException;
import ghidra.util.task.TaskMonitor;
import ghidra.util.task.TaskMonitorAdapter;
public class StructureDBTest extends AbstractGenericTest {
@ -65,7 +63,7 @@ public class StructureDBTest extends AbstractGenericTest {
private void transitionToBigEndian() {
Structure structClone = struct.clone(null);
dataMgr.remove(struct, TaskMonitor.DUMMY);
dataMgr.remove(struct);
DataOrganizationImpl dataOrg = (DataOrganizationImpl) dataMgr.getDataOrganization();
dataOrg.setBigEndian(true);
@ -1167,7 +1165,7 @@ public class StructureDBTest extends AbstractGenericTest {
"Length: 8 Alignment: 1", struct);
//@formatter:on
dataMgr.remove(dataMgr.resolve(IntegerDataType.dataType, null), TaskMonitor.DUMMY);
dataMgr.remove(dataMgr.resolve(IntegerDataType.dataType, null));
//@formatter:off
CompositeTestUtils.assertExpectedComposite(this, "/Test\n" +
@ -1208,7 +1206,7 @@ public class StructureDBTest extends AbstractGenericTest {
"Length: 11 Alignment: 1", struct);
//@formatter:on
dataMgr.remove(td, TaskMonitor.DUMMY);
dataMgr.remove(td);
//@formatter:off
CompositeTestUtils.assertExpectedComposite(this, "/Test\n" +
@ -1841,7 +1839,7 @@ public class StructureDBTest extends AbstractGenericTest {
"Length: 13 Alignment: 1", struct);
//@formatter:on
dt.getDataTypeManager().remove(dt, new TaskMonitorAdapter());
dt.getDataTypeManager().remove(dt);
//@formatter:off
CompositeTestUtils.assertExpectedComposite(this, "/Test\n" +

View file

@ -23,7 +23,6 @@ import com.google.common.collect.Sets;
import generic.test.AbstractGenericTest;
import ghidra.program.model.data.*;
import ghidra.util.task.TaskMonitor;
public class UnionDBTest extends AbstractGenericTest {
@ -59,7 +58,7 @@ public class UnionDBTest extends AbstractGenericTest {
private void transitionToBigEndian() {
Union unionClone = union.clone(null);
dataMgr.remove(union, TaskMonitor.DUMMY);
dataMgr.remove(union);
DataOrganizationImpl dataOrg = (DataOrganizationImpl) dataMgr.getDataOrganization();
dataOrg.setBigEndian(true);
@ -272,7 +271,7 @@ public class UnionDBTest extends AbstractGenericTest {
"Length: 4 Alignment: 1", union);
//@formatter:on
dataMgr.remove(td, TaskMonitor.DUMMY);
dataMgr.remove(td);
//@formatter:off
CompositeTestUtils.assertExpectedComposite(this, "/TestUnion\n" +

View file

@ -191,7 +191,12 @@ public class TestDoubleDataTypeManager implements DataTypeManager {
}
@Override
public boolean remove(DataType dataType, TaskMonitor monitor) {
public boolean remove(DataType dataType) {
throw new UnsupportedOperationException();
}
@Override
public void remove(List<DataType> dataTypes, TaskMonitor monitor) throws CancelledException {
throw new UnsupportedOperationException();
}

View file

@ -190,11 +190,16 @@ public class TestDummyDataTypeManager implements DataTypeManager {
}
@Override
public boolean remove(DataType dataType, TaskMonitor monitor) {
public boolean remove(DataType dataType) {
// stub
return false;
}
@Override
public void remove(List<DataType> dataTypes, TaskMonitor monitor) throws CancelledException {
// stub
}
@Override
public boolean contains(DataType dataType) {
// stub