Cache inlay hints to avoid flickering#2597
Conversation
This matches the naming of the `position` and `absolutePosition` methods.
e27fde2 to
c78fe43
Compare
Previously, we always recomputed the inlay hints by calling into SourceKit. If the document that we compute inlay hints for has recently changed, SourceKit may need a bit of time to recompute the semantic analysis. The inlay hints may thus need roughly 200-700ms to be computed. This causes flickering in VSCode as it removes the old inlay hints immediately and only displays them again when SourceKit-LSP returned them. With this commit we cache the inlay hints and recompute them in the background. After the recompute is finished we use `workspace/inlayHint/refresh` request to tell the client to refresh its inlay hints. On each textDocument/didChange request the cached inlay hints are shifted according to the text edits to ensure their positions are still correct. This avoids the flickering as we can always return the cached inlay hints immediately. The returned hints may however temporarily show outdated type information until the background recompute is finished.
c78fe43 to
6106d23
Compare
ahoppen
left a comment
There was a problem hiding this comment.
Thanks. Would be great to get rid of that flickering 😄
| /// Cached inlay hints for each document. | ||
| /// | ||
| /// Each entry stores hints for the full document and the document version they were computed for. | ||
| var cache: [DocumentURI: InlayHintCacheEntry] = [:] |
There was a problem hiding this comment.
I think I would prefer to make this an LRU cache similar to what we have for in the diagnostics manager and the syntax tree cache.
| let dataPosition: AbsolutePosition? | ||
| let textEdits: [ShiftedTextEdit] | ||
|
|
||
| func shiftBy(delta: SourceLength) -> ShiftedInlayHint { |
There was a problem hiding this comment.
This should be named shiftedBy to follow https://www.swift.org/documentation/api-design-guidelines/#name-according-to-side-effects.
| let task: Task<Void, any Error> | ||
| } | ||
|
|
||
| struct InlayHintState { |
There was a problem hiding this comment.
Could we have an InlayHintManager similar to how we have a SyntaxTreeManager that takes care of all the position shifting etc. I think encapsulating all of that logic in one place will make it easier to reason about.
| } catch is CancellationError { | ||
| throw CancellationError() |
There was a problem hiding this comment.
I think this doesn’t do anything.
| return | ||
| } | ||
| if latestVersion > inFlightTask.expectedVersion { | ||
| scheduleInlayHintRefresh(for: uri, expectedVersion: latestVersion) |
There was a problem hiding this comment.
Could you add a comment why this is necessary?
| preEditSnapshot: preEditSnapshot, | ||
| postEditSnapshot: postEditSnapshot | ||
| ) | ||
| scheduleInlayHintRefresh(for: notification.textDocument.uri, expectedVersion: postEditSnapshot.version) |
There was a problem hiding this comment.
Actually, I wonder if we should be re-computing inlay hints on every edit to the document if we had inlay hints cached for a previous version of the document. Chances are very high that the client will need inlay hints for the next document version as well and this way there is a chance that we’ll already have them computed when the client asks for them (or at least reduces the amount of time until they are available). What do you think?
| let updatedEntry = InlayHintCacheEntry(version: snapshot.version, hints: updatedHints) | ||
| await self.storeInlayHintCache(updatedEntry, for: uri) | ||
|
|
||
| let _ = try await self.sourceKitLSPServer?.sendRequestToClient(InlayHintRefreshRequest()) |
There was a problem hiding this comment.
My biggest concern with this approach is that this will cause the client to refresh inlay hints for all open documents, not just the one that we just re-computed the inlay hints for. I wonder if we could add an LSP extension to this request to only refresh a single document. That being said, I don’t think it’s a blocker and previous versions of inlay hints worked like this, so it’s probably fine – would just be a nice optimization.
@plemarquand Do you think it would be possible to add a middleware to the VS Code extension that captures inlayHint/refresh requests and only refreshes inlay hints for a single document as opposed to all?
| currentHints.append(contentsOf: previousHints[..<lowerBoundIndex]) | ||
| // Hints that overlap with the edit range are dropped and will be recomputed in the background. | ||
| // Hints after the edit range need to be shifted by the edit delta. | ||
| currentHints.append(contentsOf: previousHints[upperBoundIndex...].map { $0.shiftBy(delta: delta) }) |
There was a problem hiding this comment.
Just a thought: Would it be possible to do the shifting in the Position line:column space instead of the AbsolutePosition offset-based space? I think it should work as well because we just need to add something to the line number if a line was inserted before and adjust the column if there was an edit on this line. Could be missing something though.
Fixes #2468.
Comparison
Before:
Screen.Recording.2026-04-07.at.10.44.00.mov
Screen.Recording.2026-04-07.at.10.44.49.mov
After:
Screen.Recording.2026-04-07.at.10.41.16.mov
Screen.Recording.2026-04-07.at.10.45.43.mov
Description
Currently, we always recomputed the inlay hints by calling into SourceKit. If the document that we compute inlay hints for has recently changed, SourceKit may need a bit of time to perform parsing and semantic analysis. The inlay hints may thus need roughly 200-700ms to be computed. This causes flickering in VSCode as it removes the old inlay hints immediately and only displays them again when SourceKit-LSP returned them.
With this PR the inlay hints are cached and recomputed in the background. On each
textDocument/didChangerequest the cached inlay hints are shifted according to the text edits to ensure their positions are still correct. This avoids the flickering as we always have inlay hints which we can return immediately. The returned hints may however temporarily show outdated type information until the background recompute is finished. This can be seen in the first example where the inlay hints need a short time to be updated.Open Questions / ToDo
There are still a few open questions which I need to think about, but I also wanted to get input on them as early as possible:
In the current version of this PR whenever the file changes, the inlay hints for the entire file are recomputed. I think this is not strictly necessary, but I'm also unsure how much of an impact it has compared to only computing inlay hints for a part of the document. The main thing that takes time for computing inlay hints is the parsing and semantic analysis of the file in SourceKit. I haven't looked into if it can do this incrementally or if it always does the semantic analysis for the entire file.
The inlay hints for#ifdirectives are currently also cached, but I'm not sure that's necessary, they can maybe be recomputed each time.It may currently also be possible to have a memory leak with the cache if the client disconnects/loses connection without sending a
didCloserequest. This can maybe be handled by using a LRU cache or a timeout after which the cached inlay hints for a document are dropped if they were not accessed.I also don't like the constant conversions between
Positions andAbsolutePositions in the code, but they probably can't be avoided.DocumentSnapshotchangesI can also move the two commits for the
DocumentSnapshotmethods to a separate PR.