Skip to content

Commit 04d58c9

Browse files
sebthomsebthom
authored
fix: Avoid UI freezes on Open Declaration with slow lang servers (#1376)
Without the PR we currently wait up to 800ms which is for some LS too short (esp. when requests pile up) so the operation may consistently fail with: <img width="518" height="148" alt="159085032-7b63dc48-ccc6-4310-812b-56bc2456a2f9" src="https://github.com/user-attachments/assets/d9a82797-d75c-4c3a-a0c8-eaa6306b5a92" /> Since the wait happens in the UI thread the timeout cannot be extended without making the UI freeze longer. With this PR the UI thread will be blocked for max 200ms (or shorter if we agree on). If the LS does not respond in that time, a `DeferredOpenDeclarationHyperlink` instance is returned which asynchronously opens the link once LS responded. Opening the link will be dismissed if the the editor was closed in the meantime, the document modified or the response took longer than 5 seconds. Addresses #97
1 parent ff5efb6 commit 04d58c9

4 files changed

Lines changed: 478 additions & 39 deletions

File tree

‎org.eclipse.lsp4e/src/org/eclipse/lsp4e/internal/DocumentOffsetAsyncCache.java‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,141 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2025 Vegard IT GmbH and others.
3+
* This program and the accompanying materials are made
4+
* available under the terms of the Eclipse Public License 2.0
5+
* which is available at https://www.eclipse.org/legal/epl-2.0/
6+
*
7+
* SPDX-License-Identifier: EPL-2.0
8+
*
9+
* Contributors:
10+
* Sebastian Thomschke (Vegard IT GmbH) - initial implementation.
11+
*******************************************************************************/
12+
package org.eclipse.lsp4e.internal;
13+
14+
import java.time.Duration;
15+
import java.util.Collections;
16+
import java.util.Map;
17+
import java.util.WeakHashMap;
18+
import java.util.concurrent.CompletableFuture;
19+
import java.util.concurrent.ConcurrentHashMap;
20+
import java.util.concurrent.ConcurrentMap;
21+
import java.util.concurrent.TimeUnit;
22+
import java.util.function.Supplier;
23+
24+
import org.eclipse.jdt.annotation.Nullable;
25+
import org.eclipse.jface.text.IDocument;
26+
import org.eclipse.jface.text.IDocumentExtension4;
27+
28+
/**
29+
* Generic, per-document+offset cache for asynchronous results that avoids
30+
* starting the same work twice by sharing a single running task.
31+
*
32+
* <p>
33+
* Features:
34+
* <li>Weakly keys by {@link IDocument} to avoid memory leaks.
35+
* <li>Per-document concurrent maps for thread-safe access from UI and
36+
* background.
37+
* <li>Eviction: TTL-based using {@link System#nanoTime()} and document-change
38+
* invalidation when a stable modification stamp is available.
39+
* <li>In-flight de-duplication: only one running task per document+offset.
40+
* <li>Stale-result protection: if the document changes while a value is being
41+
* computed, the result is delivered to callers but is not cached.
42+
*/
43+
public final class DocumentOffsetAsyncCache<V> {
44+
45+
private record Entry<V>(V value, long createdNanos, long docModStamp) {
46+
boolean stale(final long ttlNanos, final long currentDocStamp) {
47+
return System.nanoTime() - createdNanos > ttlNanos //
48+
// Invalidate when we can confidently detect a document change
49+
|| (docModStamp != IDocumentExtension4.UNKNOWN_MODIFICATION_STAMP
50+
&& currentDocStamp != IDocumentExtension4.UNKNOWN_MODIFICATION_STAMP
51+
&& docModStamp != currentDocStamp);
52+
}
53+
}
54+
55+
private final Map<IDocument, ConcurrentMap<Integer, Entry<V>>> cache = Collections
56+
.synchronizedMap(new WeakHashMap<>());
57+
private final Map<IDocument, ConcurrentMap<Integer, CompletableFuture<V>>> inFlight = Collections
58+
.synchronizedMap(new WeakHashMap<>());
59+
60+
private final long ttlNanos;
61+
62+
public DocumentOffsetAsyncCache(final Duration ttl) {
63+
this.ttlNanos = TimeUnit.MILLISECONDS.toNanos(ttl.toMillis());
64+
}
65+
66+
/**
67+
* Returns cached value if present and valid; otherwise returns the single
68+
* running task or starts one via {@code supplier}. A value is valid if it has
69+
* not expired by TTL and (when stamps are available) matches the current
70+
* document stamp. Results computed for an older stamp are not cached.
71+
*/
72+
public CompletableFuture<V> computeIfAbsent(final IDocument doc, final int offset,
73+
final Supplier<CompletableFuture<V>> supplier) {
74+
// Fast path: return a completed future if a fresh value is already cached
75+
final @Nullable V cachedNow = getNow(doc, offset);
76+
if (cachedNow != null)
77+
return CompletableFuture.completedFuture(cachedNow);
78+
79+
final ConcurrentMap<Integer, CompletableFuture<V>> byOffset = inFlight.computeIfAbsent(doc,
80+
d -> new ConcurrentHashMap<>());
81+
return byOffset.computeIfAbsent(offset, k -> {
82+
final long startStamp = DocumentUtil.getDocumentModificationStamp(doc);
83+
final CompletableFuture<V> cf = supplier.get();
84+
cf.whenComplete((v, t) -> {
85+
// Always clean up the in-flight entry by key. Only one future exists
86+
// per offset due to computeIfAbsent, so this is safe and avoids capturing
87+
// a specific future instance.
88+
byOffset.remove(offset);
89+
if (t == null && v != null) {
90+
final long nowStamp = DocumentUtil.getDocumentModificationStamp(doc);
91+
if (startStamp == IDocumentExtension4.UNKNOWN_MODIFICATION_STAMP
92+
|| nowStamp == IDocumentExtension4.UNKNOWN_MODIFICATION_STAMP || nowStamp == startStamp) {
93+
put(doc, offset, v);
94+
}
95+
}
96+
});
97+
return cf;
98+
});
99+
}
100+
101+
/**
102+
* @return the cached value if present and valid; removes and returns null if
103+
* TTL expired or the document stamp changed.
104+
*/
105+
public @Nullable V getNow(final IDocument doc, final int offset) {
106+
final ConcurrentMap<Integer, Entry<V>> byOffset = cache.get(doc);
107+
if (byOffset == null)
108+
return null;
109+
110+
final Entry<V> e = byOffset.get(offset);
111+
if (e == null)
112+
return null;
113+
114+
final long nowStamp = DocumentUtil.getDocumentModificationStamp(doc);
115+
if (e.stale(ttlNanos, nowStamp)) {
116+
byOffset.remove(offset, e);
117+
return null;
118+
}
119+
return e.value;
120+
}
121+
122+
public void invalidate(final IDocument doc) {
123+
cache.remove(doc); // synchronizedMap handles its own locking
124+
final var map = inFlight.remove(doc); // remove returns the per-doc map, if any
125+
if (map != null) {
126+
map.values().forEach(f -> f.cancel(true));
127+
}
128+
}
129+
130+
/**
131+
* Stores a value tagged with the current document modification stamp.
132+
*/
133+
public void put(final IDocument doc, final int offset, final V value) {
134+
cache.compute(doc, (d, byOffset) -> {
135+
final ConcurrentMap<Integer, Entry<V>> map = byOffset != null ? byOffset : new ConcurrentHashMap<>();
136+
final long stamp = DocumentUtil.getDocumentModificationStamp(doc);
137+
map.put(offset, new Entry<>(value, System.nanoTime(), stamp));
138+
return map;
139+
});
140+
}
141+
}

‎org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/declaration/DeferredOpenDeclarationHyperlink.java‎

Lines changed: 118 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2025 Vegard IT GmbH and others.
3+
* This program and the accompanying materials are made
4+
* available under the terms of the Eclipse Public License 2.0
5+
* which is available at https://www.eclipse.org/legal/epl-2.0/
6+
*
7+
* SPDX-License-Identifier: EPL-2.0
8+
*
9+
* Contributors:
10+
* Sebastian Thomschke (Vegard IT GmbH) - initial implementation.
11+
*******************************************************************************/
12+
package org.eclipse.lsp4e.operations.declaration;
13+
14+
import java.util.concurrent.CompletableFuture;
15+
import java.util.concurrent.CompletionException;
16+
import java.util.concurrent.TimeUnit;
17+
18+
import org.eclipse.jdt.annotation.Nullable;
19+
import org.eclipse.jface.text.IDocument;
20+
import org.eclipse.jface.text.IDocumentExtension4;
21+
import org.eclipse.jface.text.IRegion;
22+
import org.eclipse.jface.text.ITextViewer;
23+
import org.eclipse.jface.text.hyperlink.IHyperlink;
24+
import org.eclipse.lsp4e.LanguageServerPlugin;
25+
import org.eclipse.lsp4e.internal.DocumentUtil;
26+
27+
/**
28+
* An implementation of {@link IHyperlink} which asynchronously opens the link
29+
* once the language server has responded. Opening is dismissed if the editor
30+
* was closed in the meantime, the document was modified, or the response took
31+
* longer than a given timeout.
32+
*/
33+
final class DeferredOpenDeclarationHyperlink implements IHyperlink {
34+
35+
private static final long DEFERRED_OPEN_TIMEOUT_NANOS = TimeUnit.SECONDS.toNanos(5);
36+
37+
private final ITextViewer viewer;
38+
private final IDocument document;
39+
private final long documentInitialModificationStamp;
40+
private final IRegion region;
41+
private final CompletableFuture<@Nullable IHyperlink> future;
42+
private final long createdNanos = System.nanoTime();
43+
44+
DeferredOpenDeclarationHyperlink(final ITextViewer viewer, final IDocument document, final IRegion region,
45+
final CompletableFuture<@Nullable IHyperlink> future) {
46+
this.viewer = viewer;
47+
this.document = document;
48+
this.region = region;
49+
this.future = future;
50+
this.documentInitialModificationStamp = DocumentUtil.getDocumentModificationStamp(document);
51+
}
52+
53+
@Override
54+
public IRegion getHyperlinkRegion() {
55+
return region;
56+
}
57+
58+
@Override
59+
public @Nullable String getTypeLabel() {
60+
final var link = getResolvedLink();
61+
return link != null ? link.getTypeLabel() : null;
62+
}
63+
64+
@Override
65+
public @Nullable String getHyperlinkText() {
66+
final var link = getResolvedLink();
67+
return link != null ? link.getHyperlinkText() : null;
68+
}
69+
70+
@Override
71+
public void open() {
72+
future.whenComplete((link, ex) -> {
73+
if (ex != null) {
74+
LanguageServerPlugin.logError(ex.getLocalizedMessage(), ex);
75+
return;
76+
}
77+
final var widget = viewer.getTextWidget();
78+
if (widget == null)
79+
return;
80+
if (link == null) {
81+
LanguageServerPlugin.logWarning("No hyperlink target resolved for Open Declaration"); //$NON-NLS-1$
82+
return;
83+
}
84+
widget.getDisplay().asyncExec(() -> {
85+
if (isStale())
86+
return;
87+
link.open();
88+
});
89+
});
90+
}
91+
92+
private @Nullable IHyperlink getResolvedLink() {
93+
try {
94+
return future.getNow(null);
95+
} catch (CompletionException ex) {
96+
LanguageServerPlugin.logError(ex.getLocalizedMessage(), ex);
97+
return null;
98+
}
99+
}
100+
101+
private boolean isStale() {
102+
// LS response came too late?
103+
if (System.nanoTime() - createdNanos > DEFERRED_OPEN_TIMEOUT_NANOS)
104+
return true;
105+
106+
// Editor was closed?
107+
final var widget = viewer.getTextWidget();
108+
if (widget == null || widget.isDisposed())
109+
return true;
110+
111+
// Document was modified?
112+
if (documentInitialModificationStamp != IDocumentExtension4.UNKNOWN_MODIFICATION_STAMP
113+
&& DocumentUtil.getDocumentModificationStamp(document) != documentInitialModificationStamp)
114+
return true;
115+
116+
return false;
117+
}
118+
}

‎org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/declaration/DeferredOpenMultiDeclarationHyperlink.java‎

Lines changed: 138 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,138 @@
1+
/*******************************************************************************
2+
* Copyright (c) 2025 Vegard IT GmbH and others.
3+
* This program and the accompanying materials are made
4+
* available under the terms of the Eclipse Public License 2.0
5+
* which is available at https://www.eclipse.org/legal/epl-2.0/
6+
*
7+
* SPDX-License-Identifier: EPL-2.0
8+
*
9+
* Contributors:
10+
* Sebastian Thomschke (Vegard IT GmbH) - initial implementation.
11+
******************************************************************************/
12+
package org.eclipse.lsp4e.operations.declaration;
13+
14+
import java.util.List;
15+
import java.util.concurrent.CompletableFuture;
16+
import java.util.concurrent.TimeUnit;
17+
18+
import org.eclipse.jdt.annotation.Nullable;
19+
import org.eclipse.jface.text.IDocument;
20+
import org.eclipse.jface.text.IDocumentExtension4;
21+
import org.eclipse.jface.text.IRegion;
22+
import org.eclipse.jface.text.ITextViewer;
23+
import org.eclipse.jface.text.hyperlink.IHyperlink;
24+
import org.eclipse.jface.viewers.LabelProvider;
25+
import org.eclipse.jface.window.Window;
26+
import org.eclipse.lsp4e.LanguageServerPlugin;
27+
import org.eclipse.lsp4e.internal.DocumentUtil;
28+
import org.eclipse.lsp4e.ui.Messages;
29+
import org.eclipse.swt.widgets.Shell;
30+
import org.eclipse.ui.dialogs.ElementListSelectionDialog;
31+
32+
/**
33+
* An implementation of {@link IHyperlink} which asynchronously opens a chooser
34+
* of links once the language server has responded. Opening is dismissed if the
35+
* editor was closed in the meantime, the document was modified, or the response
36+
* took longer than a given timeout.
37+
*/
38+
final class DeferredOpenMultiDeclarationHyperlink implements IHyperlink {
39+
40+
private static final long DEFERRED_OPEN_TIMEOUT_NANOS = TimeUnit.SECONDS.toNanos(5);
41+
42+
private final ITextViewer viewer;
43+
private final IDocument document;
44+
private final long documentInitialModificationStamp;
45+
private final IRegion region;
46+
private final CompletableFuture<? extends List<? extends IHyperlink>> future;
47+
private final long createdNanos = System.nanoTime();
48+
49+
DeferredOpenMultiDeclarationHyperlink(final ITextViewer viewer, final IDocument document, final IRegion region,
50+
final CompletableFuture<? extends List<? extends IHyperlink>> future) {
51+
this.viewer = viewer;
52+
this.document = document;
53+
this.region = region;
54+
this.future = future;
55+
this.documentInitialModificationStamp = DocumentUtil.getDocumentModificationStamp(document);
56+
}
57+
58+
@Override
59+
public IRegion getHyperlinkRegion() {
60+
return region;
61+
}
62+
63+
@Override
64+
public @Nullable String getTypeLabel() {
65+
return "Open Declaration (resolving...)"; //$NON-NLS-1$
66+
}
67+
68+
@Override
69+
public @Nullable String getHyperlinkText() {
70+
return "Open Declaration (resolving...)"; //$NON-NLS-1$
71+
}
72+
73+
@Override
74+
public void open() {
75+
future.whenComplete((links, ex) -> {
76+
if (ex != null) {
77+
LanguageServerPlugin.logError(ex.getLocalizedMessage(), ex);
78+
return;
79+
}
80+
final var widget = viewer.getTextWidget();
81+
if (widget == null)
82+
return;
83+
if (links.isEmpty()) {
84+
LanguageServerPlugin.logWarning("No hyperlink targets resolved for Open Declaration"); //$NON-NLS-1$
85+
return;
86+
}
87+
widget.getDisplay().asyncExec(() -> {
88+
if (isStale())
89+
return;
90+
91+
if (links.size() == 1) {
92+
links.get(0).open();
93+
return;
94+
}
95+
96+
final Shell shell = widget.getShell();
97+
final var dialog = new ElementListSelectionDialog(shell, new LabelProvider() {
98+
@Override
99+
public String getText(final @Nullable Object element) {
100+
if (element instanceof final IHyperlink link) {
101+
final String text = link.getHyperlinkText();
102+
return text != null ? text : link.getTypeLabel();
103+
}
104+
return element == null ? "" : element.toString(); //$NON-NLS-1$
105+
}
106+
});
107+
dialog.setTitle(Messages.declarationHyperlinkLabel);
108+
dialog.setMessage("Select a target:"); //$NON-NLS-1$
109+
dialog.setElements(links.toArray());
110+
dialog.setMultipleSelection(false);
111+
if (dialog.open() == Window.OK) {
112+
Object result = dialog.getFirstResult();
113+
if (result instanceof IHyperlink link) {
114+
link.open();
115+
}
116+
}
117+
});
118+
});
119+
}
120+
121+
private boolean isStale() {
122+
// LS response came too late?
123+
if (System.nanoTime() - createdNanos > DEFERRED_OPEN_TIMEOUT_NANOS)
124+
return true;
125+
126+
// Editor was closed?
127+
final var widget = viewer.getTextWidget();
128+
if (widget == null || widget.isDisposed())
129+
return true;
130+
131+
// Document was modified?
132+
if (documentInitialModificationStamp != IDocumentExtension4.UNKNOWN_MODIFICATION_STAMP
133+
&& DocumentUtil.getDocumentModificationStamp(document) != documentInitialModificationStamp)
134+
return true;
135+
136+
return false;
137+
}
138+
}

0 commit comments

Comments
 (0)