From 8eef5bd13dc40d14f11aa029533bd616c87c241f Mon Sep 17 00:00:00 2001 From: Sebastian Thomschke Date: Fri, 14 Nov 2025 22:09:18 +0100 Subject: [PATCH] perf(diagnostics): avoid loading remote documents for marker offsets Avoid calling LSPEclipseUtils.getDocument for non-file: URIs so diagnostics for remote resources no longer trigger expensive content loads just to compute marker offsets. Persist raw LSP range (start/end line/character) on diagnostic markers and use it to match existing markers when no IDocument is available, reducing unnecessary delete/recreate cycles for remote files. Keep existing behavior for local/workspace files and open editors by still computing document-based CHAR_START/CHAR_END and line numbers when a document is present. --- .../test/diagnostics/DiagnosticsTest.java | 55 ++++++++++++++ .../diagnostics/LSPDiagnosticsToMarkers.java | 75 +++++++++++++++---- 2 files changed, 114 insertions(+), 16 deletions(-) diff --git a/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/diagnostics/DiagnosticsTest.java b/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/diagnostics/DiagnosticsTest.java index 48b396496..f6674f003 100644 --- a/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/diagnostics/DiagnosticsTest.java +++ b/org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/diagnostics/DiagnosticsTest.java @@ -241,6 +241,61 @@ public void testDiagnosticsRangeAfterDocument() throws CoreException { } } + @Test + public void testDiagnosticsOnContainerResourceWithoutDocument() throws Exception { + // Use the project itself as the target resource (container, not file) so no + // IDocument is created + var projectUri = project.getLocationURI().toString(); + + var range = new Range(new Position(0, 1), new Position(0, 5)); + var diagnostic = createDiagnostic("code", "message", range, DiagnosticSeverity.Warning, "source"); + + diagnosticsToMarkers.accept(new PublishDiagnosticsParams(projectUri, List.of(diagnostic))); + + waitForAndAssertCondition(10_000, () -> { + IMarker[] markers = project.findMarkers(LSPDiagnosticsToMarkers.LS_DIAGNOSTIC_MARKER_TYPE, true, + IResource.DEPTH_ZERO); + return markers.length == 1; + }); + + IMarker[] initialMarkers = project.findMarkers(LSPDiagnosticsToMarkers.LS_DIAGNOSTIC_MARKER_TYPE, true, + IResource.DEPTH_ZERO); + assertEquals(1, initialMarkers.length); + IMarker initialMarker = initialMarkers[0]; + + // Line and LSP range attributes should come from the LSP range, not from a + // document + assertEquals(1, MarkerUtilities.getLineNumber(initialMarker)); + assertEquals(Integer.valueOf(range.getStart().getLine()), + initialMarker.getAttribute(LSPDiagnosticsToMarkers.LSP_START_LINE)); + assertEquals(Integer.valueOf(range.getStart().getCharacter()), + initialMarker.getAttribute(LSPDiagnosticsToMarkers.LSP_START_CHAR)); + assertEquals(Integer.valueOf(range.getEnd().getLine()), + initialMarker.getAttribute(LSPDiagnosticsToMarkers.LSP_END_LINE)); + assertEquals(Integer.valueOf(range.getEnd().getCharacter()), + initialMarker.getAttribute(LSPDiagnosticsToMarkers.LSP_END_CHAR)); + + // No document means no precise char-start/char-end offsets + assertEquals(-1, MarkerUtilities.getCharStart(initialMarker)); + assertEquals(-1, MarkerUtilities.getCharEnd(initialMarker)); + + // Send the same diagnostics again and ensure the existing marker is reused + // (dedup by LSP_* attributes) + long initialId = initialMarker.getId(); + diagnosticsToMarkers.accept(new PublishDiagnosticsParams(projectUri, List.of(diagnostic))); + + waitForAndAssertCondition(10_000, () -> { + IMarker[] markers = project.findMarkers(LSPDiagnosticsToMarkers.LS_DIAGNOSTIC_MARKER_TYPE, true, + IResource.DEPTH_ZERO); + return markers.length == 1; + }); + + IMarker[] markersAfterUpdate = project.findMarkers(LSPDiagnosticsToMarkers.LS_DIAGNOSTIC_MARKER_TYPE, true, + IResource.DEPTH_ZERO); + assertEquals(1, markersAfterUpdate.length); + assertEquals(initialId, markersAfterUpdate[0].getId()); + } + @Test public void testDiagnosticsFromVariousLS() throws Exception { final var content = "Diagnostic Other Text"; diff --git a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/diagnostics/LSPDiagnosticsToMarkers.java b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/diagnostics/LSPDiagnosticsToMarkers.java index 7c5d58ba1..ee513395b 100644 --- a/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/diagnostics/LSPDiagnosticsToMarkers.java +++ b/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/diagnostics/LSPDiagnosticsToMarkers.java @@ -12,6 +12,7 @@ *******************************************************************************/ package org.eclipse.lsp4e.operations.diagnostics; +import java.net.URI; import java.util.ArrayList; import java.util.HashMap; import java.util.HashSet; @@ -46,7 +47,6 @@ import org.eclipse.lsp4e.internal.ArrayUtil; import org.eclipse.lsp4j.Diagnostic; import org.eclipse.lsp4j.PublishDiagnosticsParams; -import org.eclipse.lsp4j.Range; import org.eclipse.lsp4j.jsonrpc.messages.Either; import org.eclipse.ui.IEditorReference; import org.eclipse.ui.texteditor.MarkerUtilities; @@ -57,6 +57,13 @@ public class LSPDiagnosticsToMarkers implements Consumer targetAttributes, IMarker marker } private @Nullable IMarker getExistingMarkerFor(@Nullable IDocument document, Diagnostic diagnostic, Set remainingMarkers) { - if (document == null) { - return null; - } - final var markerMessage = markerAttributeComputer.computeMarkerMessage(diagnostic); + final var rangeStart = diagnostic.getRange().getStart(); + final var rangeEnd = diagnostic.getRange().getEnd(); for (IMarker marker : remainingMarkers) { if (!marker.exists()) { continue; } try { - if (LSPEclipseUtils.toOffset(diagnostic.getRange().getStart(), document) == MarkerUtilities.getCharStart(marker) - && (LSPEclipseUtils.toOffset(diagnostic.getRange().getEnd(), document) == MarkerUtilities.getCharEnd(marker) || Objects.equals(diagnostic.getRange().getStart(), diagnostic.getRange().getEnd())) - && Objects.equals(marker.getAttribute(IMarker.MESSAGE), markerMessage) - && Objects.equals(marker.getAttribute(LANGUAGE_SERVER_ID), this.languageServerId)) { - return marker; + // Always require same server and same user-visible message + if (!languageServerId.equals(marker.getAttribute(LANGUAGE_SERVER_ID)) + || !markerMessage.equals(marker.getAttribute(IMarker.MESSAGE))) { + continue; + } + if (document != null) { + // Document available: match by precise character offsets + final int startOff = LSPEclipseUtils.toOffset(rangeStart, document); + final int endOff = LSPEclipseUtils.toOffset(rangeEnd, document); + if (startOff == MarkerUtilities.getCharStart(marker) + && (endOff == MarkerUtilities.getCharEnd(marker) || rangeStart.equals(rangeEnd))) { + return marker; + } + } else { + // No document: match by raw LSP range attributes stored on the marker + final int markerStartLine = marker.getAttribute(LSP_START_LINE, Integer.MIN_VALUE); + final int markerStartChar = marker.getAttribute(LSP_START_CHAR, Integer.MIN_VALUE); + final int markerEndLine = marker.getAttribute(LSP_END_LINE, Integer.MIN_VALUE); + final int markerEndChar = marker.getAttribute(LSP_END_CHAR, Integer.MIN_VALUE); + if (markerStartLine == Integer.MIN_VALUE || markerEndLine == Integer.MIN_VALUE) { + continue; + } + final boolean sameStart = markerStartLine == rangeStart.getLine() + && markerStartChar == rangeStart.getCharacter(); + final boolean sameEnd = markerEndLine == rangeEnd.getLine() + && markerEndChar == rangeEnd.getCharacter(); + if (sameStart && (sameEnd || rangeStart.equals(rangeEnd))) { + return marker; + } } } catch (CoreException | BadLocationException e) { LanguageServerPlugin.logError(e); @@ -255,24 +291,31 @@ private Map computeMarkerAttributes(@Nullable IDocument document if (source != null) { diagnostic.setSource(source.intern()); } - final var attributes = new HashMap(8); + final var attributes = new HashMap(12); attributes.put(LSP_DIAGNOSTIC, diagnostic); attributes.put(LANGUAGE_SERVER_ID, languageServerId); attributes.put(IMarker.MESSAGE, markerAttributeComputer.computeMarkerMessage(diagnostic)); attributes.put(IMarker.SEVERITY, LSPEclipseUtils.toEclipseMarkerSeverity(diagnostic.getSeverity())); + final var rangeStart = diagnostic.getRange().getStart(); + final var rangeEnd = diagnostic.getRange().getEnd(); + attributes.put(IMarker.LINE_NUMBER, rangeStart.getLine() + 1); + attributes.put(LSP_START_LINE, rangeStart.getLine()); + attributes.put(LSP_START_CHAR, rangeStart.getCharacter()); + attributes.put(LSP_END_LINE, rangeEnd.getLine()); + attributes.put(LSP_END_CHAR, rangeEnd.getCharacter()); + if (document != null) { - Range range = diagnostic.getRange(); int documentLength = document.getLength(); int start; try { - start = Math.min(LSPEclipseUtils.toOffset(range.getStart(), document), documentLength); + start = Math.min(LSPEclipseUtils.toOffset(rangeStart, document), documentLength); } catch (BadLocationException ex) { start = documentLength; } int end; try { - end = Math.min(LSPEclipseUtils.toOffset(range.getEnd(), document), documentLength); + end = Math.min(LSPEclipseUtils.toOffset(rangeEnd, document), documentLength); } catch (BadLocationException ex) { end = documentLength; }