Skip to content

Commit 9dfdf10

Browse files
sebthomrubenporras
sebthom
authored and
rubenporras
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 ed07757 commit 9dfdf10

File tree

2 files changed

+114
-16
lines changed

2 files changed

+114
-16
lines changed

‎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
@@ -241,6 +241,61 @@ public void testDiagnosticsRangeAfterDocument() throws CoreException {
241241
}
242242
}
243243

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