Skip to content

Commit 95a4eed

Browse files
sebthomsebthom
committed
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.
1 parent fb5de3e commit 95a4eed

2 files changed

Lines changed: 114 additions & 16 deletions

File tree

‎org.eclipse.lsp4e.test/src/org/eclipse/lsp4e/test/diagnostics/DiagnosticsTest.java‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,61 @@ public void testDiagnosticsRangeAfterDocument() throws CoreException {
239239
}
240240
}
241241

242+
@Test
243+
public void testDiagnosticsOnContainerResourceWithoutDocument() throws Exception {
244+
// Use the project itself as the target resource (container, not file) so no
245+
// IDocument is created
246+
var projectUri = project.getLocationURI().toString();
247+
248+
var range = new Range(new Position(0, 1), new Position(0, 5));
249+
var diagnostic = createDiagnostic("code", "message", range, DiagnosticSeverity.Warning, "source");
250+
251+
diagnosticsToMarkers.accept(new PublishDiagnosticsParams(projectUri, List.of(diagnostic)));
252+
253+
waitForAndAssertCondition(10_000, () -> {
254+
IMarker[] markers = project.findMarkers(LSPDiagnosticsToMarkers.LS_DIAGNOSTIC_MARKER_TYPE, true,
255+
IResource.DEPTH_ZERO);
256+
return markers.length == 1;
257+
});
258+
259+
IMarker[] initialMarkers = project.findMarkers(LSPDiagnosticsToMarkers.LS_DIAGNOSTIC_MARKER_TYPE, true,
260+
IResource.DEPTH_ZERO);
261+
assertEquals(1, initialMarkers.length);
262+
IMarker initialMarker = initialMarkers[0];
263+
264+
// Line and LSP range attributes should come from the LSP range, not from a
265+
// document
266+
assertEquals(1, MarkerUtilities.getLineNumber(initialMarker));
267+
assertEquals(Integer.valueOf(range.getStart().getLine()),
268+
initialMarker.getAttribute(LSPDiagnosticsToMarkers.LSP_START_LINE));
269+
assertEquals(Integer.valueOf(range.getStart().getCharacter()),
270+
initialMarker.getAttribute(LSPDiagnosticsToMarkers.LSP_START_CHAR));
271+
assertEquals(Integer.valueOf(range.getEnd().getLine()),
272+
initialMarker.getAttribute(LSPDiagnosticsToMarkers.LSP_END_LINE));
273+
assertEquals(Integer.valueOf(range.getEnd().getCharacter()),
274+
initialMarker.getAttribute(LSPDiagnosticsToMarkers.LSP_END_CHAR));
275+
276+
// No document means no precise char-start/char-end offsets
277+
assertEquals(-1, MarkerUtilities.getCharStart(initialMarker));
278+
assertEquals(-1, MarkerUtilities.getCharEnd(initialMarker));
279+
280+
// Send the same diagnostics again and ensure the existing marker is reused
281+
// (dedup by LSP_* attributes)
282+
long initialId = initialMarker.getId();
283+
diagnosticsToMarkers.accept(new PublishDiagnosticsParams(projectUri, List.of(diagnostic)));
284+
285+
waitForAndAssertCondition(10_000, () -> {
286+
IMarker[] markers = project.findMarkers(LSPDiagnosticsToMarkers.LS_DIAGNOSTIC_MARKER_TYPE, true,
287+
IResource.DEPTH_ZERO);
288+
return markers.length == 1;
289+
});
290+
291+
IMarker[] markersAfterUpdate = project.findMarkers(LSPDiagnosticsToMarkers.LS_DIAGNOSTIC_MARKER_TYPE, true,
292+
IResource.DEPTH_ZERO);
293+
assertEquals(1, markersAfterUpdate.length);
294+
assertEquals(initialId, markersAfterUpdate[0].getId());
295+
}
296+
242297
@Test
243298
public void testDiagnosticsFromVariousLS() throws Exception {
244299
final var content = "Diagnostic Other Text";

‎org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/diagnostics/LSPDiagnosticsToMarkers.java‎

Lines changed: 59 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
*******************************************************************************/
1313
package org.eclipse.lsp4e.operations.diagnostics;
1414

15+
import java.net.URI;
1516
import java.util.ArrayList;
1617
import java.util.HashMap;
1718
import java.util.HashSet;
@@ -46,7 +47,6 @@
4647
import org.eclipse.lsp4e.internal.ArrayUtil;
4748
import org.eclipse.lsp4j.Diagnostic;
4849
import org.eclipse.lsp4j.PublishDiagnosticsParams;
49-
import org.eclipse.lsp4j.Range;
5050
import org.eclipse.lsp4j.jsonrpc.messages.Either;
5151
import org.eclipse.ui.IEditorReference;
5252
import org.eclipse.ui.texteditor.MarkerUtilities;
@@ -57,6 +57,13 @@ public class LSPDiagnosticsToMarkers implements Consumer<PublishDiagnosticsParam
5757
public static final String LANGUAGE_SERVER_ID = "languageServerId"; //$NON-NLS-1$
5858
public static final String LS_DIAGNOSTIC_MARKER_TYPE = "org.eclipse.lsp4e.diagnostic"; //$NON-NLS-1$
5959

60+
// LSP range attributes stored on markers to allow matching without computing
61+
// document offsets
62+
public static final String LSP_START_LINE = "lspStartLine"; //$NON-NLS-1$
63+
public static final String LSP_START_CHAR = "lspStartChar"; //$NON-NLS-1$
64+
public static final String LSP_END_LINE = "lspEndLine"; //$NON-NLS-1$
65+
public static final String LSP_END_CHAR = "lspEndChar"; //$NON-NLS-1$
66+
6067
private static final IMarkerAttributeComputer DEFAULT_MARKER_ATTRIBUTE_COMPUTER = new IMarkerAttributeComputer() {
6168

6269
@Override
@@ -165,8 +172,15 @@ private void doRun() throws CoreException {
165172
// when we're done
166173
IDocument existingDocument = LSPEclipseUtils.getExistingDocument(resource);
167174
final boolean hasDiagnostics = !diagnostics.getDiagnostics().isEmpty();
168-
final boolean temporaryLoadDocument = existingDocument == null;
169-
IDocument document = (hasDiagnostics && temporaryLoadDocument) ? LSPEclipseUtils.getDocument(resource): existingDocument;
175+
boolean temporaryLoadDocument = false;
176+
IDocument document = existingDocument;
177+
if (hasDiagnostics && document == null) {
178+
final @Nullable URI resourceUri = LSPEclipseUtils.toUri(resource);
179+
if (resourceUri != null && "file".equals(resourceUri.getScheme())) { //$NON-NLS-1$
180+
temporaryLoadDocument = true;
181+
document = LSPEclipseUtils.getDocument(resource);
182+
}
183+
}
170184
for (Diagnostic diagnostic : diagnostics.getDiagnostics()) {
171185
IMarker associatedMarker = getExistingMarkerFor(document, diagnostic, toDeleteMarkers);
172186
if (associatedMarker == null) {
@@ -222,21 +236,43 @@ protected void updateMarker(Map<String, Object> targetAttributes, IMarker marker
222236
}
223237

224238
private @Nullable IMarker getExistingMarkerFor(@Nullable IDocument document, Diagnostic diagnostic, Set<IMarker> remainingMarkers) {
225-
if (document == null) {
226-
return null;
227-
}
228-
229239
final var markerMessage = markerAttributeComputer.computeMarkerMessage(diagnostic);
240+
final var rangeStart = diagnostic.getRange().getStart();
241+
final var rangeEnd = diagnostic.getRange().getEnd();
230242
for (IMarker marker : remainingMarkers) {
231243
if (!marker.exists()) {
232244
continue;
233245
}
234246
try {
235-
if (LSPEclipseUtils.toOffset(diagnostic.getRange().getStart(), document) == MarkerUtilities.getCharStart(marker)
236-
&& (LSPEclipseUtils.toOffset(diagnostic.getRange().getEnd(), document) == MarkerUtilities.getCharEnd(marker) || Objects.equals(diagnostic.getRange().getStart(), diagnostic.getRange().getEnd()))
237-
&& Objects.equals(marker.getAttribute(IMarker.MESSAGE), markerMessage)
238-
&& Objects.equals(marker.getAttribute(LANGUAGE_SERVER_ID), this.languageServerId)) {
239-
return marker;
247+
// Always require same server and same user-visible message
248+
if (!languageServerId.equals(marker.getAttribute(LANGUAGE_SERVER_ID))
249+
|| !markerMessage.equals(marker.getAttribute(IMarker.MESSAGE))) {
250+
continue;
251+
}
252+
if (document != null) {
253+
// Document available: match by precise character offsets
254+
final int startOff = LSPEclipseUtils.toOffset(rangeStart, document);
255+
final int endOff = LSPEclipseUtils.toOffset(rangeEnd, document);
256+
if (startOff == MarkerUtilities.getCharStart(marker)
257+
&& (endOff == MarkerUtilities.getCharEnd(marker) || rangeStart.equals(rangeEnd))) {
258+
return marker;
259+
}
260+
} else {
261+
// No document: match by raw LSP range attributes stored on the marker
262+
final int markerStartLine = marker.getAttribute(LSP_START_LINE, Integer.MIN_VALUE);
263+
final int markerStartChar = marker.getAttribute(LSP_START_CHAR, Integer.MIN_VALUE);
264+
final int markerEndLine = marker.getAttribute(LSP_END_LINE, Integer.MIN_VALUE);
265+
final int markerEndChar = marker.getAttribute(LSP_END_CHAR, Integer.MIN_VALUE);
266+
if (markerStartLine == Integer.MIN_VALUE || markerEndLine == Integer.MIN_VALUE) {
267+
continue;
268+
}
269+
final boolean sameStart = markerStartLine == rangeStart.getLine()
270+
&& markerStartChar == rangeStart.getCharacter();
271+
final boolean sameEnd = markerEndLine == rangeEnd.getLine()
272+
&& markerEndChar == rangeEnd.getCharacter();
273+
if (sameStart && (sameEnd || rangeStart.equals(rangeEnd))) {
274+
return marker;
275+
}
240276
}
241277
} catch (CoreException | BadLocationException e) {
242278
LanguageServerPlugin.logError(e);
@@ -255,24 +291,31 @@ private Map<String, Object> computeMarkerAttributes(@Nullable IDocument document
255291
if (source != null) {
256292
diagnostic.setSource(source.intern());
257293
}
258-
final var attributes = new HashMap<String, Object>(8);
294+
final var attributes = new HashMap<String, Object>(12);
259295
attributes.put(LSP_DIAGNOSTIC, diagnostic);
260296
attributes.put(LANGUAGE_SERVER_ID, languageServerId);
261297
attributes.put(IMarker.MESSAGE, markerAttributeComputer.computeMarkerMessage(diagnostic));
262298
attributes.put(IMarker.SEVERITY, LSPEclipseUtils.toEclipseMarkerSeverity(diagnostic.getSeverity()));
263299

300+
final var rangeStart = diagnostic.getRange().getStart();
301+
final var rangeEnd = diagnostic.getRange().getEnd();
302+
attributes.put(IMarker.LINE_NUMBER, rangeStart.getLine() + 1);
303+
attributes.put(LSP_START_LINE, rangeStart.getLine());
304+
attributes.put(LSP_START_CHAR, rangeStart.getCharacter());
305+
attributes.put(LSP_END_LINE, rangeEnd.getLine());
306+
attributes.put(LSP_END_CHAR, rangeEnd.getCharacter());
307+
264308
if (document != null) {
265-
Range range = diagnostic.getRange();
266309
int documentLength = document.getLength();
267310
int start;
268311
try {
269-
start = Math.min(LSPEclipseUtils.toOffset(range.getStart(), document), documentLength);
312+
start = Math.min(LSPEclipseUtils.toOffset(rangeStart, document), documentLength);
270313
} catch (BadLocationException ex) {
271314
start = documentLength;
272315
}
273316
int end;
274317
try {
275-
end = Math.min(LSPEclipseUtils.toOffset(range.getEnd(), document), documentLength);
318+
end = Math.min(LSPEclipseUtils.toOffset(rangeEnd, document), documentLength);
276319
} catch (BadLocationException ex) {
277320
end = documentLength;
278321
}

0 commit comments

Comments
 (0)