Bug 2691 - Doing multiple calls for marker attributes (1GI5Q1G)
Summary: Doing multiple calls for marker attributes (1GI5Q1G)
Status: RESOLVED INVALID
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 2.0   Edit
Hardware: All Windows NT
: P2 normal (vote)
Target Milestone: ---   Edit
Assignee: Randy Giffen CLA
QA Contact:
URL:
Whiteboard:
Keywords: performance
Depends on:
Blocks:
 
Reported: 2001-10-10 22:41 EDT by Tod Creasey CLA
Modified: 2002-05-31 23:09 EDT (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tod Creasey CLA 2001-10-10 22:41:35 EDT
When we build the Tasks View from a restarted Eclipse Image we call get Attribute on a Marker
about 10-12 times per marker. This is an expensive operation as it requires file system access 
each time.

Note that the amount of calls to getAttribute is the same no matter if you are creating the resource or java view
or if you are starting up or opening a new perspective.

First of all I have built a Marker sorter

package org.eclipse.ui.views.tasklist;

import java.util.Hashtable;
import org.eclipse.core.resources.IMarker;
import org.eclipse.core.runtime.CoreException;
import org.eclipse.core.runtime.IPath;

/**
 * The MarkerAttributeCache is the class that holds onto the attributes
 * for markers to prevent constant file I/O access for operations
 * that query the same information several times like sorting.
 */

/*package*/
class MarkerAttributeCache {

	private Hashtable messages = new Hashtable();
	private Hashtable lineNumbers = new Hashtable();
	private Hashtable locations = new Hashtable();
	private Hashtable severities = new Hashtable();
	private Hashtable priorities = new Hashtable();
	private Hashtable containerNames = new Hashtable();
	private Hashtable types = new Hashtable();
	private Hashtable completion = new Hashtable();
	private static String[] attributeNames =
		{
			IMarker.MESSAGE,
			IMarker.LINE_NUMBER,
			IMarker.LOCATION,
			IMarker.SEVERITY,
			IMarker.DONE,
			IMarker.PRIORITY };

	public MarkerAttributeCache() {
	}

	/* 
	 * Get all of the attributes relevant to marker and cache them
	 * for later lookup
	 */
	private void getAttributes(IMarker marker) {

		Object[] attributes = new Object[6];
		try {
			attributes = marker.getAttributes(attributeNames);
		} catch (CoreException excption) {
			//There has been an exception - populate with defaults
			messages.put(marker, "");
			lineNumbers.put(marker, new Integer(-1));
			locations.put(marker, "");
			severities.put(marker, new Integer(IMarker.SEVERITY_WARNING));
			completion.put(marker, new Boolean(false));
			priorities.put(marker, new Integer(IMarker.PRIORITY_NORMAL));
			return;
		}
	
		addEntry(messages,marker,attributes[0],"");
		addEntry(lineNumbers,marker,attributes[1],new Integer(-1));
		addEntry(locations,marker,attributes[2],"");
		addEntry(severities,marker,attributes[3],new Integer(IMarker.SEVERITY_WARNING));
		addEntry(completion,marker,attributes[4],new Boolean(false));
		addEntry(priorities,marker,attributes[5],new Integer(IMarker.PRIORITY_NORMAL));

	}
	
	/**
	 * Add the value to the table keyed on the marker. If that
	 * value is null add the default instead
	 */ 
	private void addEntry(Hashtable table, IMarker marker, Object value, Object defaultValue){
		if(value == null)
			table.put(marker,defaultValue);
		else
			table.put(marker,value);
	}		

	public String getMessage(IMarker marker) {
		if (!this.messages.containsKey(marker))
			getAttributes(marker);
		return (String) messages.get(marker);

	}

	/**
	* Returns the line number of the given marker.
	*/
	public int getLineNumber(IMarker marker) {
		if (!this.lineNumbers.containsKey(marker))
			getAttributes(marker);
		return ((Integer) lineNumbers.get(marker)).intValue();
	}
	/**
	 * Returns the text for the location field.
	 */
	public String getLocation(IMarker marker) {
		if (!this.locations.containsKey(marker))
			getAttributes(marker);
		return (String) locations.get(marker);
	}

	/**
	* Returns the severity of the given marker.  Default is SEVERITY_WARNING.
	*/
	public int getSeverity(IMarker marker) {
		if (!this.severities.containsKey(marker))
			getAttributes(marker);
		return ((Integer) severities.get(marker)).intValue();

	}

	/**
	* Returns the priority of the given marker.  Default is PRIORITY_NORMAL.
	*/
	public int getPriority(IMarker marker) {
		if (!this.priorities.containsKey(marker))
			getAttributes(marker);
		return ((Integer) priorities.get(marker)).intValue();
	}

	/**
	* Returns the sort order for the given marker based on its completion status.
	* Lower numbers appear first.
	*/
	public int getCompletedOrder(IMarker marker) {

		if (!this.completion.containsKey(marker))
			getAttributes(marker);
		return ((Boolean) completion.get(marker)).booleanValue() ? 0 : 1;
	}

	public String getContainerName(IMarker marker) {

		if (!containerNames.contains(marker)) {
			// taking substring from resource's path string is 40x faster 
			// than getting relative path string from resource's parent
			String path = marker.getResource().getFullPath().toString();
			int i = path.lastIndexOf(IPath.SEPARATOR);
			String parentName;
			if (i == 0)
				parentName = ""; //$NON-NLS-1$
			else
				parentName = path.substring(1, i);
			containerNames.put(marker, parentName);
		}
		return (String) containerNames.get(marker);
	}

	/**
	* Returns the text for the line and location column.
	*/
	public String getLineAndLocation(IMarker marker) {
		int lineNumber = getLineNumber(marker);
		String location = getLocation(marker);
		if (lineNumber == -1) {
			if (location.equals("")) { //$NON-NLS-1$
				return ""; //$NON-NLS-1$
			} else {
				return location;
			}
		} else {
			if (location.equals("")) { //$NON-NLS-1$
				return TaskListMessages.format(
					"TaskList.line",
					new Object[] { new Integer(lineNumber)});
				//$NON-NLS-1$
			} else {
				return TaskListMessages.format(
					"TaskList.lineAndLocation",
					new Object[] { new Integer(lineNumber), location });
				//$NON-NLS-1$
			}
		}
	}

	/**
	* Returns whether the given marker is of the given type (either directly or indirectly).
	*/
	public boolean isMarkerType(IMarker marker, String type) {
		if (type.equals(types.get(marker)))
			return true;
		try {
			return marker.isSubtypeOf(type);
		} catch (CoreException e) {
			return false;
		}
	}

}

which is used wth this version of the TaskSorter

package org.eclipse.ui.views.tasklist;

/*
 * (c) Copyright IBM Corp. 2000, 2001.
 * All Rights Reserved.
 */
import org.eclipse.core.resources.IMarker;
import org.eclipse.jface.viewers.*;
import java.text.Collator;
import java.util.Hashtable;

/**
 * This is the abstract superclass of sorters in the task list.
 */
/* package */ class TaskSorter extends ViewerSorter {
	private TaskList tasklist;
	private boolean reversed = false;
	private int columnNumber;
	private MarkerAttributeCache markerCache;
	
	private static final int NUM_COLUMNS = 7;
	
	// column headings:	"","C", "!","Description","Resource Name", "In Folder", "Location"
	private static final int[][] SORT_ORDERS_BY_COLUMN = {
		{0, 2, 4, 5, 6, 3, 1},	/* category */ 
		{1, 0, 2, 4, 5, 6, 3},	/* completed */
		{2, 0, 4, 5, 6, 3, 1},	/* priority */
		{3, 4, 5, 6, 0, 2, 1},	/* description */
		{4, 5, 6, 3, 0, 2, 1},	/* resource */
		{5, 4, 6, 3, 0, 2, 1},	/* container */
		{6, 4, 5, 3, 0, 2, 1} 	/* location */
	};
/**
 * Creates a new task sorter.
 */
public TaskSorter(TaskList tasklist, int columnNumber) {
	this.tasklist = tasklist;
	this.columnNumber = columnNumber;
}
/* (non-Javadoc)
 * Method declared on ViewerSorter.
 */
/**
 * Compares two markers, sorting first by the main column of this sorter,
 * then by subsequent columns, depending on the column sort order.
 */
public int compare(Viewer viewer, Object e1, Object e2) {
	IMarker m1 = (IMarker) e1;
	IMarker m2 = (IMarker) e2;
	int[] columnSortOrder = SORT_ORDERS_BY_COLUMN[columnNumber];
	int result = 0;
	for (int i = 0; i < NUM_COLUMNS; ++i) {
		result = compareColumnValue(columnSortOrder[i], m1, m2);
		if (result != 0)
			break;
	}
	if (reversed)
		result = -result;
	return result;
}
/* (non-Javadoc)
 * Method declared on ViewerSorter.
 */
/**
 * Compares two markers, based only on the value of the specified column.
 */
private int compareColumnValue(int columnNumber, IMarker m1, IMarker m2) {
	switch (columnNumber) {
		case 0: /* category */
			return getCategoryOrder(m1) - getCategoryOrder(m2);
		case 1: /* completed */
			return getCompletedOrder(m1) - getCompletedOrder(m2);
		case 2: /* priority */
			return getPriorityOrder(m1) - getPriorityOrder(m2);
		case 3: /* description */
			return collator.compare(markerCache.getMessage(m1), markerCache.getMessage(m2));
		case 4: /* resource name */
			// Optimization: if the markers' resources are equal, then their names are the same.
			// If resources are equal, chances are they're identical; don't take hit for full equality comparison.
			if (m1.getResource() == m2.getResource())
				return 0;
			return collator.compare(MarkerUtil.getResourceName(m1), MarkerUtil.getResourceName(m2));
		case 5: /* container name */
			// Optimization: if the markers' resources are equal, then container names are the same.
			// If resources are equal, chances are they're identical; don't take hit for full equality comparison.
			if (m1.getResource() == m2.getResource())
				return 0;
			return collator.compare(markerCache.getContainerName(m1), markerCache.getContainerName(m2));
		case 6: /* line and location */
			return compareLineAndLocation(m1, m2);
		default:
			return 0;
	}
}

/**
 * Compares the line number and location of the two markers.
 * If line number is specified for both, this sorts first by line number (numerically), then by location (textually).
 * If line number is not specified for either, this sorts by location.
 * Otherwise, if only one has a line number, this sorts by the combined text for line number and location.
 */
private int compareLineAndLocation(IMarker m1, IMarker m2) {
	int line1 = markerCache.getLineNumber(m1);
	int line2 = markerCache.getLineNumber(m2);
	if (line1 != -1 && line2 != -1) {
		if (line1 != line2) {
			return line1 - line2;
		}
		else {
			String loc1 = markerCache.getLocation(m1);
			String loc2 = markerCache.getLocation(m2);
			return collator.compare(loc1, loc2);
		}
	}
	if (line1 == -1 && line2 == -1) {
		String loc1 = markerCache.getLocation(m1);
		String loc2 = markerCache.getLocation(m2);
		return collator.compare(loc1, loc2);
	}
	String loc1 = markerCache.getLineAndLocation(m1);
	String loc2 = markerCache.getLineAndLocation(m2);
	return collator.compare(loc1, loc2);
}
/**
 * Returns the sort order for the given marker based on its category.
 * Lower numbers appear first.
 */
private int getCategoryOrder(IMarker marker) {
	if (markerCache.isMarkerType(marker, IMarker.PROBLEM)) {
		switch (markerCache.getSeverity(marker)) {
			case IMarker.SEVERITY_ERROR:
				return 1;
			case IMarker.SEVERITY_WARNING:
				return 2;
			case IMarker.SEVERITY_INFO:
				return 3;
		}
	} else if (MarkerUtil.isMarkerType(marker, IMarker.TASK)) {
		return 0;
	}
	return 1000;
}
/**
 * Returns the number of the column by which this is sorting.
 */
public int getColumnNumber() {
	return columnNumber;
}
/**
 * Returns the sort order for the given marker based on its completion status.
 * Lower numbers appear first.
 */
private int getCompletedOrder(IMarker marker) {
	return markerCache.getCompletedOrder(marker);
}
/**
 * Returns the sort order for the given marker based on its priority.
 * Lower numbers appear first.
 */
private int getPriorityOrder(IMarker marker) {
	// want HIGH to appear first
	return IMarker.PRIORITY_HIGH - markerCache.getPriority(marker);
}
/**
 * Returns true for descending, or false
 * for ascending sorting order.
 */
public boolean isReversed() {
	return reversed;
}
/**
 * Sets the sorting order.
 */
public void setReversed(boolean newReversed) {
	reversed = newReversed;
}

/* Sort the elements from viewer. Use the defualt method
* but initialize a cache for the values before it starts
*/

public void sort(final Viewer viewer, Object[] elements) {
	this.markerCache = new MarkerAttributeCache();
	super.sort(viewer,elements);
	this.markerCache = null;
}
}


NOTES:

TC (10/2/2001 3:10:08 PM) - this change needs to be reveiwed harder with the current implementation.,
It currently demonstrates errors and should not be integrated until the 2.0 stream is more stable
Comment 1 DJ Houghton CLA 2001-10-29 19:10:00 EST
PRODUCT VERSION: 0.9

Comment 2 Tod Creasey CLA 2002-01-22 10:23:31 EST
This has not been applied as of 20020115
Comment 3 Nick Edgar CLA 2002-05-31 23:09:59 EDT
This is stale.  There have been several performance improvements to the tasks 
view, including reducing the number of calls to findMarker to 1.