Keep marker attribute access off the UI thread in the markers view

bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/ExtendedMarkersView.java

@@ -167,6 +167,9 @@ public class ExtendedMarkersView extends ViewPart {
 	 */
 	private static final String TAG_SHOW_FILTER_TEXT = "showFilterText"; //$NON-NLS-1$
 
+	private static final MessageFormat SUMMARY_BREAKDOWN = new MessageFormat(
+			MarkerMessages.errorsAndWarningsSummaryBreakdown);
+
 	private final IMarker[] noMarkers = new IMarker[0];
 
 	private MarkerContentGenerator generator;
@@ -973,9 +976,7 @@ private String getStatusMessage(Markers markers, Integer[] counts) {
 			}
 			return status;
 		}
-		String message= MessageFormat.format(
-				MarkerMessages.errorsAndWarningsSummaryBreakdown,
-				counts[0], counts[1], /* combine infos and others */ counts[2] + counts[3]);
+		String message = formatSummaryBreakdown(counts);
 		if (filteredCount < 0 || filteredCount >= totalCount) {
 			return message;
 		}
@@ -1506,8 +1507,15 @@ private String getStatusSummary(MarkerEntry[] entries) {
 		}
 		return MessageFormat.format(MarkerMessages.marker_statusSummarySelected, entries.length,
 				/* combine infos and others */
-				MessageFormat.format(MarkerMessages.errorsAndWarningsSummaryBreakdown, counts[0], counts[1],
-						counts[2] + counts[3]));
+				formatSummaryBreakdown(counts));
+	}
+
+	/**
+	 * Formats the "n errors, n warnings, n others" summary; the parsed pattern is
+	 * reused since it is needed on every update.
+	 */
+	private static String formatSummaryBreakdown(Integer[] counts) {
+		return SUMMARY_BREAKDOWN.format(new Object[] { counts[0], counts[1], counts[2] + counts[3] });
 	}
 
 	/**

bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/MarkerEntry.java

@@ -70,7 +70,11 @@ public Class<?>[] getAdapterList() {
 	private static final Object CACHED_NULL = new String("CACHED_NULL"); //$NON-NLS-1$
 	private MarkerCategory category;
 	private final Map<String, Object> cache = new ConcurrentHashMap<>();
+	// RuleBasedCollator.getCollationKey is synchronized, so one shared instance is safe.
+	private static final Collator COLLATOR = Collator.getInstance();
+	// Previous update's keys are kept for one round; only the update job touches these.
 	private static Map<String, CollationKey> collationCache = new ConcurrentHashMap<>();
+	private static Map<String, CollationKey> previousCollationCache = new ConcurrentHashMap<>();
 
 	/**
 	 * Set the MarkerEntry to be stale, if discovered at any point of time
@@ -207,9 +211,10 @@ CollationKey getCollationKey(String attribute, String defaultValue) {
 		if (attributeValue.isEmpty()) {
 			return MarkerSupportInternalUtilities.EMPTY_COLLATION_KEY;
 		}
-		CollationKey key = collationCache.computeIfAbsent(attributeValue,
-				k -> Collator.getInstance().getCollationKey(attributeValue));
-		return key;
+		return collationCache.computeIfAbsent(attributeValue, k -> {
+			CollationKey previous = previousCollationCache.get(k);
+			return previous != null ? previous : COLLATOR.getCollationKey(k);
+		});
 	}
 
 	@Override
@@ -366,6 +371,7 @@ void clearCache() {
 	}
 
 	static void clearCollationCache() {
+		previousCollationCache = collationCache;
 		collationCache = new ConcurrentHashMap<>();
 	}
 

bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/MarkerSortUtil.java

@@ -103,7 +103,6 @@ private static void adjustMaxElement(MarkerEntry[] heapArray, int first, int hea
 
 			++current;
 		}
-		MarkerEntry.clearCollationCache();
 	}
 
 	/**
@@ -289,7 +288,6 @@ public static void sortStartingKElement(MarkerEntry[] entries,
 			for (int i = from; i <= to; i++) {
 				entries[i].clearCache();
 			}
-			MarkerEntry.clearCollationCache();
 			return;
 		}
 

bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/Markers.java

@@ -94,6 +94,8 @@ synchronized boolean updateWithNewMarkers(Collection<MarkerEntry> markerEntries,
 			MarkerEntry[] markerArray = new MarkerEntry[markerEntries.size()];
 			markerEntries.toArray(markerArray);
 			markerEntryArray = markerArray;
+			// Count here, off the UI thread; this also primes the severity cache for the sort.
+			markerCounts = getMarkerCounts(markerArray);
 			if (sortAndGroup) {
 				if (monitor.isCanceled()) {
 					return false;
@@ -189,6 +191,9 @@ synchronized boolean sortMarkerEntries(IProgressMonitor monitor) {
 			return false;
 		} finally {
 			inChange = initialVal;
+			// Rotate once per complete sort, not per category, so the previous
+			// update's keys are still there for every category of the next one.
+			MarkerEntry.clearCollationCache();
 		}
 	}
 
@@ -304,17 +309,7 @@ Integer[] getMarkerCounts() {
 	static Integer[] getMarkerCounts(MarkerEntry[] entries) {
 		int[] ints = new int[] { 0, 0, 0, 0 };
 		for (MarkerEntry entry : entries) {
-			IMarker marker = entry.getMarker();
-			int severity = -1;
-			Object value = null;
-			try {
-				value = marker.getAttribute(IMarker.SEVERITY);
-			} catch (CoreException e) {
-				entry.checkIfMarkerStale();
-			}
-			if (value instanceof Integer) {
-				severity = ((Integer) value).intValue();
-			}
+			int severity = entry.getAttributeValue(IMarker.SEVERITY) instanceof Integer value ? value.intValue() : -1;
 			if (severity >= IMarker.SEVERITY_INFO) {
 				ints[severity]++;
 			} else {
@@ -381,6 +376,7 @@ Markers getClone() {
 		if (!inChange) {
 			markers.markerEntryArray = markerEntryArray.clone();
 			markers.categories = categories.clone();
+			markers.markerCounts = markerCounts;
 		}
 		return markers;
 	}