[228774] Improve ElementComparer Performance

rse/plugins/org.eclipse.rse.core/src/org/eclipse/rse/internal/core/model/SystemRegistry.java

@@ -48,6 +48,7 @@
  * David McKnight   (IBM)        - [224313] [api] Create RSE Events for MOVE and COPY holding both source and destination fields
  * David Dykstal (IBM) - [200735][Persistence] Delete a profile that contains a connection and restart, profile is back without connections
  * David Dykstal (IBM) - [168976][api] move ISystemNewConnectionWizardPage from core to UI
+ * Martin Oberhuber (Wind River) - [228774] Improve ElementComparer Performance
  ********************************************************************************/
 
 package org.eclipse.rse.internal.core.model;
@@ -60,6 +61,7 @@ import java.util.Set;
 import java.util.Vector;
 
 import org.eclipse.core.runtime.IAdaptable;
+import org.eclipse.core.runtime.IAdapterManager;
 import org.eclipse.core.runtime.IProgressMonitor;
 import org.eclipse.core.runtime.IStatus;
 import org.eclipse.core.runtime.Platform;
@@ -88,9 +90,9 @@ import org.eclipse.rse.core.filters.ISystemFilterStartHere;
 import org.eclipse.rse.core.filters.SystemFilterReference;
 import org.eclipse.rse.core.model.IHost;
 import org.eclipse.rse.core.model.ISubSystemConfigurationCategories;
+import org.eclipse.rse.core.model.ISubSystemConfigurator;
 import org.eclipse.rse.core.model.ISystemContainer;
 import org.eclipse.rse.core.model.ISystemHostPool;
-import org.eclipse.rse.core.model.ISubSystemConfigurator;
 import org.eclipse.rse.core.model.ISystemProfile;
 import org.eclipse.rse.core.model.ISystemProfileManager;
 import org.eclipse.rse.core.model.ISystemRegistry;
@@ -880,95 +882,93 @@ public class SystemRegistry implements ISystemRegistry
 	}
 
 	/**
-	 * Check if two objects refers to the same system object by comparing it absoluteName with its subsystem id.
+	 * Adapt the given element to an adapter that allows reading the element's
+	 * absolute name and parent subsystem.
+	 *
+	 * @param element an element to adapt.
+	 * @return the requested adapter, or <code>null</code> if the element is
+	 *         not adaptable as needed.
+	 * @since org.eclipse.rse.core 3.0
+	 */
+	public static ISystemDragDropAdapter getSystemDragDropAdapter(Object element) {
+		Object adapter = null;
+		if (element instanceof IAdaptable) {
+			IAdaptable adaptable = (IAdaptable) element;
+			adapter = adaptable.getAdapter(ISystemDragDropAdapter.class);
+			if (adapter == null) {
+				adapter = Platform.getAdapterManager().getAdapter(element, "org.eclipse.rse.ui.view.ISystemViewElementAdapter"); //$NON-NLS-1$
+				if (adapter == null) {
+					return null;
+				}
+				assert false : "Found ISystemViewElementAdapter but no ISystemDragDropAdapter"; //$NON-NLS-1$
+			}
+		} else {
+			IAdapterManager am = Platform.getAdapterManager();
+			adapter = am.getAdapter(element, ISystemDragDropAdapter.class.getName());
+			if (adapter == null) {
+				adapter = am.getAdapter(element, "org.eclipse.rse.ui.view.ISystemViewElementAdapter"); //$NON-NLS-1$
+				if (adapter == null) {
+					return null;
+				}
+				assert false : "Found ISystemViewElementAdapter but no ISystemDragDropAdapter"; //$NON-NLS-1$
+			}
+		}
+		// At this point, we know for sure that we can adapt!
+		return (ISystemDragDropAdapter) adapter;
+	}
+
+	/**
+	 * Check if two objects refers to the same system object by comparing their
+	 * absolute Names and subsystem id's.
 	 *
 	 * @param firstObject the first object to compare
-	 * @param firstObjectFullName the full name of the firstObject.  If null, get the full name from the firstObject
+	 * @param firstObjectFullName the full name of the firstObject. If null, get
+	 *            the full name from the firstObject
 	 * @param secondObject the second object to compare
-	 * @param secondObjectFullName the full name of the secondObject. If null, get the full name from the secondObject
+	 * @param secondObjectFullName the full name of the secondObject. If null,
+	 *            get the full name from the secondObject
+	 * @return <code>true</code> if the objects to be compared are the same
+	 *         instance; or, if both objects are non-null and adaptable to an
+	 *         RSE ISystemDragDropAdapter each, and those adapters do return a
+	 *         valid absolute name that's the same for both elements, and both
+	 *         elements belong to the same subsystem instance. Otherwise,
+	 *         <code>false</code> in all other cases.
 	 */
-	public boolean isSameObjectByAbsoluteName(Object firstObject, String firstObjectFullName, Object secondObject, String secondObjectFullName)
+	public static boolean isSameObjectByAbsoluteName(Object firstObject, String firstObjectFullName, Object secondObject, String secondObjectFullName)
-	{
-		if (firstObject == secondObject)
 	{
+		if (firstObject == secondObject) {
 			return true;
 		}
-		String firstObjectAbsoluteNameWithSubSystemId = null;
+		ISystemDragDropAdapter adA = null;
-		
+		ISystemDragDropAdapter adB = null;
-		//Simply doing comparason of if two object is equal is not enough
+		if (firstObjectFullName == null) {
-		//If two different objects, but if their absoluate path (with subsystem id)
+			adA = getSystemDragDropAdapter(firstObject);
-		//are the same, they refer to the same remote object.
+			if (adA != null) {
-		
+				firstObjectFullName = adA.getAbsoluteName(firstObject);
-		if(firstObject instanceof IAdaptable) 
-		{
-			ISystemDragDropAdapter adapter = null;
-	    	
-			adapter = (ISystemDragDropAdapter)((IAdaptable)firstObject).getAdapter(ISystemDragDropAdapter.class);
-			String subSystemId = null; 
-      	
-      	  	if (adapter != null ) {
-      		  // first need to check subsystems
-      		  ISubSystem subSystem = adapter.getSubSystem(firstObject);
-      		  if (null != subSystem)
-      		  {
-      			  subSystemId = getAbsoluteNameForSubSystem(subSystem);
-      		  }
-      		  else
-      		  {
-      			  subSystemId = "";  //$NON-NLS-1$
-      		  }
-      		
-      		  if (firstObjectFullName != null)
-      		  {
-      			firstObjectAbsoluteNameWithSubSystemId = subSystemId + ":" + firstObjectFullName; //$NON-NLS-1$
-      		  }
-      		  else
-      		  {
-      			  String absolutePath = adapter.getAbsoluteName(firstObject);
-      			  firstObjectAbsoluteNameWithSubSystemId = subSystemId + ":" + absolutePath;  //$NON-NLS-1$
-      		  }
-      			 
 			}
 		}
-		
+		if (secondObjectFullName == null) {
-		
+			adB = getSystemDragDropAdapter(secondObject);
-		String secondObjectAbsoluteNameWithSubSystemId = null;
+			if (adB != null) {
-		if(secondObject instanceof IAdaptable) 
+				secondObjectFullName = adB.getAbsoluteName(secondObject);
-		{
-			ISystemDragDropAdapter adapter = null;
-	    	
-			adapter = (ISystemDragDropAdapter)((IAdaptable)secondObject).getAdapter(ISystemDragDropAdapter.class);
-			String subSystemId = null; 
-      	
-      	  	if (adapter != null ) {
-      		  // first need to check subsystems
-      		  ISubSystem subSystem = adapter.getSubSystem(secondObject);
-      		  if (null != subSystem)
-    		  {
-    			  subSystemId = getAbsoluteNameForSubSystem(subSystem);
-    		  }
-    		  else
-    		  {
-    			  subSystemId = "";  //$NON-NLS-1$
-    		  }
-      		  if (secondObjectFullName != null)
-    		  {
-    			secondObjectAbsoluteNameWithSubSystemId = subSystemId + ":" + secondObjectFullName; //$NON-NLS-1$
-    		  }
-    		  else
-    		  {
-    			  String absolutePath = adapter.getAbsoluteName(secondObject);
-    			  secondObjectAbsoluteNameWithSubSystemId = subSystemId + ":" + absolutePath;  //$NON-NLS-1$
-    		  }
-      			 
 			}
 		}
-		
+		if (firstObjectFullName != null && firstObjectFullName.equals(secondObjectFullName)) {
-		if (firstObjectAbsoluteNameWithSubSystemId != null && firstObjectAbsoluteNameWithSubSystemId.equals(secondObjectAbsoluteNameWithSubSystemId))
+			// full names exist and are the same: compare the subsystems
-		{
+			if (adA == null) { // firstFullName was passed in
-			return true;
+				adA = getSystemDragDropAdapter(firstObject);
+				assert adA != null : "full name \"" + firstObjectFullName + "\" has no ISystemDragDropAdapter!"; //$NON-NLS-1$ //$NON-NLS-2$
+				assert firstObjectFullName.equals(adA.getAbsoluteName(firstObject)) : "full name \"" + firstObjectFullName + "\" differs from adapter response: " + adA.getAbsoluteName(firstObject); //$NON-NLS-1$ //$NON-NLS-2$
+			}
+			if (adB == null) { // secondFullName was passed in
+				adB = getSystemDragDropAdapter(secondObject);
+				assert adB != null : "full name \"" + secondObjectFullName + "\" has no ISystemDragDropAdapter!"; //$NON-NLS-1$ //$NON-NLS-2$
+				assert secondObjectFullName.equals(adB.getAbsoluteName(secondObject)) : "full name \"" + firstObjectFullName + "\" differs from adapter response: " + adB.getAbsoluteName(secondObject); //$NON-NLS-1$ //$NON-NLS-2$
+			}
+			ISubSystem ssA = adA.getSubSystem(firstObject);
+			ISubSystem ssB = adB.getSubSystem(secondObject);
+			return ssA == ssB;
 		}
-		
 		return false;
 	}
 
@@ -984,7 +984,7 @@ public class SystemRegistry implements ISystemRegistry
 		String connectionName = connection.getAliasName();
 
 		dataStream.append(profileName);
-		dataStream.append("."); //$NON-NLS-1$
+		dataStream.append(".");
 		dataStream.append(connectionName);
 		return dataStream.toString();
 	}
@@ -1200,7 +1200,7 @@ public class SystemRegistry implements ISystemRegistry
 			if (conns != null) {
 				for (int jdx = 0; jdx < conns.length; jdx++) {
 					//ISystemHostPool ensures that we never have "null" hosts.
-					assert conns[jdx]!=null : "Null host in pool "+pools[idx].getName()+" at "+jdx; //$NON-NLS-1$ //$NON-NLS-2$
+					assert conns[jdx]!=null : "Null host in pool "+pools[idx].getName()+" at "+jdx;
 					hosts.add(conns[jdx]);
 				}
 			}
@@ -1445,7 +1445,7 @@ public class SystemRegistry implements ISystemRegistry
 			}
 		}
 		if ((systemType != null) && (systemType.isLocal() && (v.size() == 0)))
-			v.addElement("localhost"); //$NON-NLS-1$
+			v.addElement("localhost");
 		return (String[])v.toArray(new String[v.size()]);
 	}
 
@@ -1468,7 +1468,7 @@ public class SystemRegistry implements ISystemRegistry
 		  			profile.getName(), localType,
 		  			name, // connection name
 		  			"localhost", // hostname //$NON-NLS-1$
-		  			"", // description //$NON-NLS-1$
+		  			"", // description
 		  			// DY:  defect 42101, description cannot be null
 		  			// null, // description
 		  			userId, // default user Id
@@ -1477,7 +1477,7 @@ public class SystemRegistry implements ISystemRegistry
 		}
 		catch (Exception exc)
 		{
-			RSECorePlugin.getDefault().getLogger().logError("Error creating local connection", exc); //$NON-NLS-1$
+			RSECorePlugin.getDefault().getLogger().logError("Error creating local connection", exc);
 		}
 		return localConn;
 	}
@@ -1896,13 +1896,13 @@ public class SystemRegistry implements ISystemRegistry
 		}
 		catch (SystemMessageException exc)
 		{
-			RSECorePlugin.getDefault().getLogger().logError("Exception in updateConnection for " + connectionName, exc); //$NON-NLS-1$
+			RSECorePlugin.getDefault().getLogger().logError("Exception in updateConnection for " + connectionName, exc);
 			lastException = exc;
 			return;
 		}
 		catch (Exception exc)
 		{
-			RSECorePlugin.getDefault().getLogger().logError("Exception in updateConnection for " + connectionName, exc); //$NON-NLS-1$
+			RSECorePlugin.getDefault().getLogger().logError("Exception in updateConnection for " + connectionName, exc);
 			lastException = exc;
 			return;
 		}
@@ -2042,7 +2042,7 @@ public class SystemRegistry implements ISystemRegistry
 		ISystemHostPool targetPool = getHostPool(targetProfile);
 		IHost newConn = null;
 
-		RSECorePlugin.getDefault().getLogger().logDebugMessage(this.getClass().getName(), "Start of system connection copy. From: " + oldName + " to: " + newName); //$NON-NLS-1$ //$NON-NLS-2$
+		RSECorePlugin.getDefault().getLogger().logDebugMessage(this.getClass().getName(), "Start of system connection copy. From: " + oldName + " to: " + newName);
 
 		// STEP 0: BRING ALL IMPACTED SUBSYSTEM FACTORIES TO LIFE NOW, BEFORE DOING THE CLONE.
 		getSubSystemFactories(conn);
@@ -2061,7 +2061,7 @@ public class SystemRegistry implements ISystemRegistry
 
 			ISubSystem[] subsystems = null;
 			ISubSystemConfiguration factory = null;
-			msg = "Copying subsystems for connection " + conn.getAliasName(); //$NON-NLS-1$
+			msg = "Copying subsystems for connection " + conn.getAliasName();
 			//monitor.subTask(msg);
 			RSECorePlugin.getDefault().getLogger().logDebugMessage(this.getClass().getName(), msg);
 			subsystems = getSubSystems(conn); // get old subsystems for this connection
@@ -2069,7 +2069,7 @@ public class SystemRegistry implements ISystemRegistry
 			{
 				for (int jdx = 0; jdx < subsystems.length; jdx++)
 				{
-					msg += ": subsystem " + subsystems[jdx].getName(); //$NON-NLS-1$
+					msg += ": subsystem " + subsystems[jdx].getName();
 					//monitor.subTask(msg);
 					RSECorePlugin.getDefault().getLogger().logDebugMessage(this.getClass().getName(), msg);
 					factory = subsystems[jdx].getSubSystemConfiguration();
@@ -2094,11 +2094,11 @@ public class SystemRegistry implements ISystemRegistry
 			}
 			catch (Exception exc)
 			{
-				RSECorePlugin.getDefault().getLogger().logError("Exception (ignored) cleaning up from copy-connection exception.", exc); //$NON-NLS-1$
+				RSECorePlugin.getDefault().getLogger().logError("Exception (ignored) cleaning up from copy-connection exception.", exc);
 			}
 			throw (lastExc);
 		}
-		RSECorePlugin.getDefault().getLogger().logDebugMessage(this.getClass().getName(), "Copy of system connection " + oldName + " to " + newName + " successful"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+		RSECorePlugin.getDefault().getLogger().logDebugMessage(this.getClass().getName(), "Copy of system connection " + oldName + " to " + newName + " successful");
 		if (getSystemProfileManager().isSystemProfileActive(targetProfile.getName()))
 		{
 			int eventType = ISystemResourceChangeEvents.EVENT_ADD_RELATIVE;
@@ -2128,7 +2128,7 @@ public class SystemRegistry implements ISystemRegistry
 			if (newConn != null)
 			{
 				deleteHost(conn); // delete old connection now that new one created successfully
-				RSECorePlugin.getDefault().getLogger().logDebugMessage(this.getClass().getName(), "Move of system connection " + conn.getAliasName() + " to profile " + targetProfile.getName() + " successful"); //$NON-NLS-1$ //$NON-NLS-2$ //$NON-NLS-3$
+				RSECorePlugin.getDefault().getLogger().logDebugMessage(this.getClass().getName(), "Move of system connection " + conn.getAliasName() + " to profile " + targetProfile.getName() + " successful");
 				fireEvent(new SystemResourceChangeEvent(conn, ISystemResourceChangeEvents.EVENT_DELETE, this));
 			}
 		}
@@ -2268,7 +2268,7 @@ public class SystemRegistry implements ISystemRegistry
 				}
 				catch (InterruptedException exc)
 				{
-					System.out.println("Cancelled"); //$NON-NLS-1$
+					System.out.println("Cancelled");
 					cancelled = true;
 				}
 				catch (Exception exc)

rse/plugins/org.eclipse.rse.ui/UI/org/eclipse/rse/internal/ui/view/ElementComparer.java

@@ -15,29 +15,24 @@
  * Martin Oberhuber (Wind River) - [215820] Move SystemRegistry implementation to Core
  * David Dykstal (IBM) - [225911] Exception received after deleting a profile containing a connection
  * David Dykstal (IBM) - [228774] [regression] AssertionFailedException when connecting to New Connection
+ * Martin Oberhuber (Wind River) - [228774] Improve ElementComparer Performance
  ********************************************************************************/
 
 package org.eclipse.rse.internal.ui.view;
 
-import org.eclipse.core.runtime.IAdaptable;
 import org.eclipse.jface.viewers.IElementComparer;
-import org.eclipse.rse.core.RSECorePlugin;
+import org.eclipse.rse.core.subsystems.ISystemDragDropAdapter;
-import org.eclipse.rse.core.model.ISystemRegistry;
 import org.eclipse.rse.internal.core.model.SystemRegistry;
-import org.eclipse.rse.ui.view.ISystemViewElementAdapter;
 
 /**
- * An implememtation of an element comparer for the system view.
+ * An implementation of an element comparer for the system view.
  */
 public class ElementComparer implements IElementComparer {
 
 	public boolean equals(Object a, Object b) {
-		boolean result = false;
+		// equal if same absolute name in same subsystem;
-		ISystemRegistry registry = RSECorePlugin.getTheSystemRegistry();
+		// or, when adapters are not found, both are the same instance.
-		if (registry instanceof SystemRegistry) {
+		return SystemRegistry.isSameObjectByAbsoluteName(a, null, b, null);
-			result = ((SystemRegistry) registry).isSameObjectByAbsoluteName(a, null, b, null);
-		}
-		return result;
 	}
 
 	public int hashCode(Object element) {
@@ -54,19 +49,26 @@ public class ElementComparer implements IElementComparer {
 		 */
 		int result = 0;
 		if (element != null) {
-			if (element instanceof IAdaptable) {
+			ISystemDragDropAdapter dda = SystemRegistry.getSystemDragDropAdapter(element);
-				ISystemViewElementAdapter ident = (ISystemViewElementAdapter) ((IAdaptable) element).getAdapter(ISystemViewElementAdapter.class);
+			if (dda != null) {
-				if (ident != null) {
+				// adapter available
-					String absName = ident.getAbsoluteName(element);
+				String absName = dda.getAbsoluteName(element);
 				if (absName != null) {
 					result = absName.hashCode();
 				} else {
-						result = ident.hashCode();
+					result = dda.hashCode();
-					}
-				} else {
-					result = element.hashCode();
 				}
 			} else {
+				// --MOB: Usually, we should fall back to constant hashcode 0
+				// here if no adapter is available, in order to ensure constant
+				// hashcode even if object properties change. But as a matter of
+				// fact, those elements that we have in the SystemView and which
+				// do not have an adapter registered, are very few; and they are
+				// always constant over their lifetime, such as the "Pending..."
+				// node for instance. We therefore return the element's hashcode
+				// here, along with the corresponding equals() code above,
+				// which falls back to Object equality if no adapter is
+				// available.
 				result = element.hashCode();
 			}
 		}

rse/plugins/org.eclipse.rse.ui/UI/org/eclipse/rse/internal/ui/view/SystemTableViewPart.java

@@ -31,7 +31,8 @@
  * David McKnight   (IBM)        - [225506] [api][breaking] RSE UI leaks non-API types
  * Xuan Chen        (IBM)        - [225685] NPE when running archive testcases
  * David McKnight   (IBM)        - [225506] [api][breaking] RSE UI leaks non-API type
- ********************************************************************************/
+ * Martin Oberhuber (Wind River) - [228774] Improve ElementComparer Performance
+*******************************************************/
 
 package org.eclipse.rse.internal.ui.view;
 
@@ -1670,15 +1671,10 @@ public class SystemTableViewPart extends ViewPart
 
 		Object input = _viewer.getInput();
 
-		ISystemRegistry registry = RSECorePlugin.getTheSystemRegistry();
-		
-		boolean referToSameObject = false;
-		if (registry instanceof SystemRegistry)
-		{			
 		String[] oldNames = event.getOldNames();
-			String oldName = (oldNames == null)? null : oldNames[0];
+		// right now assuming only one resource
-			referToSameObject = ((SystemRegistry)registry).isSameObjectByAbsoluteName(input, null, child, oldName); // right now assuming only one resource
+		String oldName = (oldNames == null) ? null : oldNames[0];
-		}
+		boolean referToSameObject = SystemRegistry.isSameObjectByAbsoluteName(input, null, child, oldName);
 
 		if (input == child || child instanceof java.util.List || referToSameObject)
 		{