From 32528c3a640fff94c3859be721120d3fbf1d9963 Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Mon, 31 Aug 2026 07:48:09 +0200 Subject: [PATCH] Keep marker attribute access off the UI thread in the markers view MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Profiling a rebuild showed three avoidable costs. The severity counts for the title were computed on the UI thread by calling IMarker.getAttribute for every entry again, throwing a ResourceException for each marker deleted since the gather; they are now computed on the update job and carried into the clone. Collation keys for sorting were recomputed after every update with a fresh Collator per miss; one Collator is shared and the previous update's keys are kept for one round. The summary message pattern was re-parsed by MessageFormat on every title update; it is now parsed once. Assisted-by: multiple AI agents and layers of automated tooling 🤖 --- .../views/markers/ExtendedMarkersView.java | 18 +++++++++++++----- .../ui/internal/views/markers/MarkerEntry.java | 12 +++++++++--- .../internal/views/markers/MarkerSortUtil.java | 2 -- .../ui/internal/views/markers/Markers.java | 18 +++++++----------- 4 files changed, 29 insertions(+), 21 deletions(-) diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/ExtendedMarkersView.java b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/ExtendedMarkersView.java index f5ec3cf8c5f..943fa521b11 100644 --- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/ExtendedMarkersView.java +++ b/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] }); } /** diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/MarkerEntry.java b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/MarkerEntry.java index 09890af9947..a728b7ba4c8 100644 --- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/MarkerEntry.java +++ b/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 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 collationCache = new ConcurrentHashMap<>(); + private static Map 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<>(); } diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/MarkerSortUtil.java b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/MarkerSortUtil.java index b45070d6634..111d1e6416f 100644 --- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/MarkerSortUtil.java +++ b/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; } diff --git a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/Markers.java b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/Markers.java index cb28a0e9cac..ad65de17572 100644 --- a/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/Markers.java +++ b/bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/views/markers/Markers.java @@ -94,6 +94,8 @@ synchronized boolean updateWithNewMarkers(Collection 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; }