Skip to content

Commit 38fba69

Browse files
committed
Add cache to avoid requesting diagnostics from sourcekitd frequently
Move the responsibility of sending requests to get diagnostics from SwiftLanguageServer to DiagnosticReportManager Make buildSettings as part of the key to the cache Add ReportTask and refine the code Reset cache for reportTasks if needed Finetune the wordings and logics Solve conflicts Only remove necessary items from cache
1 parent e8e4fb2 commit 38fba69

File tree

4 files changed

+218
-75
lines changed

4 files changed

+218
-75
lines changed

Sources/SourceKitLSP/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ target_sources(SourceKitLSP PRIVATE
2222
Swift/CommentXML.swift
2323
Swift/CursorInfo.swift
2424
Swift/Diagnostic.swift
25+
Swift/DiagnosticReportManager.swift
2526
Swift/DocumentSymbols.swift
2627
Swift/EditorPlaceholder.swift
2728
Swift/FoldingRange.swift
Lines changed: 183 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,183 @@
1+
//===----------------------------------------------------------------------===//
2+
//
3+
// This source file is part of the Swift.org open source project
4+
//
5+
// Copyright (c) 2014 - 2020 Apple Inc. and the Swift project authors
6+
// Licensed under Apache License v2.0 with Runtime Library Exception
7+
//
8+
// See https://swift.org/LICENSE.txt for license information
9+
// See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors
10+
//
11+
//===----------------------------------------------------------------------===//
12+
13+
import LSPLogging
14+
import LanguageServerProtocol
15+
import SourceKitD
16+
import SwiftParserDiagnostics
17+
18+
actor DiagnosticReportManager {
19+
/// A task to produce diagnostics, either from a diagnostics request to `sourcektid` or by using the built-in swift-syntax.
20+
private typealias ReportTask = Task<RelatedFullDocumentDiagnosticReport, Error>
21+
22+
private let sourcekitd: SourceKitD
23+
private let syntaxTreeManager: SyntaxTreeManager
24+
private let documentManager: DocumentManager
25+
private let clientHasDiagnosticsCodeDescriptionSupport: Bool
26+
27+
private nonisolated var keys: sourcekitd_keys { return sourcekitd.keys }
28+
private nonisolated var requests: sourcekitd_requests { return sourcekitd.requests }
29+
30+
/// The cache that stores reportTasks for snapshot id and buildSettings
31+
///
32+
/// Conceptually, this is a dictionary. To prevent excessive memory usage we
33+
/// only keep `cacheSize` entries within the array. Older entries are at the
34+
/// end of the list, newer entries at the front.
35+
private var reportTaskCache:
36+
[(
37+
snapshotID: DocumentSnapshot.ID,
38+
buildSettings: SwiftCompileCommand?,
39+
reportTask: ReportTask
40+
)] = []
41+
42+
/// The number of reportTasks to keep
43+
///
44+
/// - Note: This has been chosen without scientific measurements.
45+
private let cacheSize = 5
46+
47+
init(
48+
sourcekitd: SourceKitD,
49+
syntaxTreeManager: SyntaxTreeManager,
50+
documentManager: DocumentManager,
51+
clientHasDiagnosticsCodeDescriptionSupport: Bool
52+
) {
53+
self.sourcekitd = sourcekitd
54+
self.syntaxTreeManager = syntaxTreeManager
55+
self.documentManager = documentManager
56+
self.clientHasDiagnosticsCodeDescriptionSupport = clientHasDiagnosticsCodeDescriptionSupport
57+
}
58+
59+
func diagnosticReport(
60+
for snapshot: DocumentSnapshot,
61+
buildSettings: SwiftCompileCommand?
62+
) async throws -> RelatedFullDocumentDiagnosticReport {
63+
if let reportTask = reportTask(for: snapshot.id, buildSettings: buildSettings) {
64+
return try await reportTask.value
65+
}
66+
let reportTask: Task<RelatedFullDocumentDiagnosticReport, Error>
67+
if let buildSettings, !buildSettings.isFallback {
68+
reportTask = Task {
69+
return try await requestReport(with: snapshot, compilerArgs: buildSettings.compilerArgs)
70+
}
71+
} else {
72+
logger.log(
73+
"Producing syntactic diagnostics from the built-in swift-syntax because we \(buildSettings != nil ? "have fallback build settings" : "don't have build settings", privacy: .public))"
74+
)
75+
// If we don't have build settings or we only have fallback build settings,
76+
// sourcekitd won't be able to give us accurate semantic diagnostics.
77+
// Fall back to providing syntactic diagnostics from the built-in
78+
// swift-syntax. That's the best we can do for now.
79+
reportTask = Task {
80+
return try await requestFallbackReport(with: snapshot)
81+
}
82+
}
83+
setReportTask(for: snapshot.id, buildSettings: buildSettings, reportTask: reportTask)
84+
return try await reportTask.value
85+
}
86+
87+
func removeItemsFromCache(with uri: DocumentURI) async {
88+
for item in reportTaskCache {
89+
if item.snapshotID.uri == uri {
90+
item.reportTask.cancel()
91+
}
92+
}
93+
reportTaskCache.removeAll(where: { $0.snapshotID.uri == uri })
94+
}
95+
96+
private func requestReport(
97+
with snapshot: DocumentSnapshot,
98+
compilerArgs: [String]
99+
) async throws -> LanguageServerProtocol.RelatedFullDocumentDiagnosticReport {
100+
try Task.checkCancellation()
101+
102+
let keys = self.keys
103+
104+
let skreq = sourcekitd.dictionary([
105+
keys.request: requests.diagnostics,
106+
keys.sourcefile: snapshot.uri.pseudoPath,
107+
keys.compilerargs: compilerArgs as [SKDValue],
108+
])
109+
110+
let dict = try await self.sourcekitd.send(skreq, fileContents: snapshot.text)
111+
112+
try Task.checkCancellation()
113+
guard (try? documentManager.latestSnapshot(snapshot.uri).id) == snapshot.id else {
114+
// Check that the document wasn't modified while we were getting diagnostics. This could happen because we are
115+
// calling `fullDocumentDiagnosticReport` from `publishDiagnosticsIfNeeded` outside of `messageHandlingQueue`
116+
// and thus a concurrent edit is possible while we are waiting for the sourcekitd request to return a result.
117+
throw ResponseError.unknown("Document was modified while loading diagnostics")
118+
}
119+
120+
var diagnostics: [Diagnostic] = []
121+
dict[keys.diagnostics]?.forEach { _, diag in
122+
if let diag = Diagnostic(
123+
diag,
124+
in: snapshot,
125+
useEducationalNoteAsCode: self.clientHasDiagnosticsCodeDescriptionSupport
126+
) {
127+
diagnostics.append(diag)
128+
}
129+
return true
130+
}
131+
132+
return RelatedFullDocumentDiagnosticReport(items: diagnostics)
133+
}
134+
135+
private func requestFallbackReport(
136+
with snapshot: DocumentSnapshot
137+
) async throws -> LanguageServerProtocol.RelatedFullDocumentDiagnosticReport {
138+
// If we don't have build settings or we only have fallback build settings,
139+
// sourcekitd won't be able to give us accurate semantic diagnostics.
140+
// Fall back to providing syntactic diagnostics from the built-in
141+
// swift-syntax. That's the best we can do for now.
142+
let syntaxTree = await syntaxTreeManager.syntaxTree(for: snapshot)
143+
let swiftSyntaxDiagnostics = ParseDiagnosticsGenerator.diagnostics(for: syntaxTree)
144+
let diagnostics = swiftSyntaxDiagnostics.compactMap { (diag) -> Diagnostic? in
145+
if diag.diagnosticID == StaticTokenError.editorPlaceholder.diagnosticID {
146+
// Ignore errors about editor placeholders in the source file, similar to how sourcekitd ignores them.
147+
return nil
148+
}
149+
return Diagnostic(diag, in: snapshot)
150+
}
151+
return RelatedFullDocumentDiagnosticReport(items: diagnostics)
152+
}
153+
154+
/// The reportTask for the given document snapshot and buildSettings.
155+
private func reportTask(
156+
for snapshotID: DocumentSnapshot.ID,
157+
buildSettings: SwiftCompileCommand?
158+
) -> ReportTask? {
159+
return reportTaskCache.first(where: { $0.snapshotID == snapshotID && $0.buildSettings == buildSettings })?
160+
.reportTask
161+
}
162+
163+
/// Set the reportTask for the given document snapshot and buildSettings.
164+
///
165+
/// If we are already storing `cacheSize` many reports, the oldest one
166+
/// will get discarded.
167+
private func setReportTask(
168+
for snapshotID: DocumentSnapshot.ID,
169+
buildSettings: SwiftCompileCommand?,
170+
reportTask: ReportTask
171+
) {
172+
reportTaskCache.insert((snapshotID, buildSettings, reportTask), at: 0)
173+
174+
// Remove any reportTasks for old versions of this document.
175+
reportTaskCache.removeAll(where: { $0.snapshotID < snapshotID })
176+
177+
// If we still have more than `cacheSize` reportTasks, delete the ones that
178+
// were produced last. We can always re-request them on-demand.
179+
while reportTaskCache.count > cacheSize {
180+
reportTaskCache.removeLast()
181+
}
182+
}
183+
}

Sources/SourceKitLSP/Swift/SwiftLanguageServer.swift

Lines changed: 28 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,8 @@ public actor SwiftLanguageServer: ToolchainLanguageServer {
139139

140140
private var stateChangeHandlers: [(_ oldState: LanguageServerState, _ newState: LanguageServerState) -> Void] = []
141141

142+
private let diagnosticReportManager: DiagnosticReportManager
143+
142144
/// Creates a language server for the given client using the sourcekitd dylib specified in `toolchain`.
143145
/// `reopenDocuments` is a closure that will be called if sourcekitd crashes and the `SwiftLanguageServer` asks its parent server to reopen all of its documents.
144146
/// Returns `nil` if `sourcektid` couldn't be found.
@@ -157,6 +159,12 @@ public actor SwiftLanguageServer: ToolchainLanguageServer {
157159
self.state = .connected
158160
self.generatedInterfacesPath = options.generatedInterfacesPath.asURL
159161
try FileManager.default.createDirectory(at: generatedInterfacesPath, withIntermediateDirectories: true)
162+
self.diagnosticReportManager = DiagnosticReportManager(
163+
sourcekitd: self.sourcekitd,
164+
syntaxTreeManager: syntaxTreeManager,
165+
documentManager: documentManager,
166+
clientHasDiagnosticsCodeDescriptionSupport: capabilityRegistry.clientHasDiagnosticsCodeDescriptionSupport
167+
)
160168
}
161169

162170
/// - Important: For testing only
@@ -270,6 +278,7 @@ extension SwiftLanguageServer {
270278

271279
private func reopenDocument(_ snapshot: DocumentSnapshot, _ compileCmd: SwiftCompileCommand?) async {
272280
cancelInFlightPublishDiagnosticsTask(for: snapshot.uri)
281+
await diagnosticReportManager.removeItemsFromCache(with: snapshot.id.uri)
273282

274283
let keys = self.keys
275284
let path = snapshot.uri.pseudoPath
@@ -317,6 +326,7 @@ extension SwiftLanguageServer {
317326

318327
public func openDocument(_ note: DidOpenTextDocumentNotification) async {
319328
cancelInFlightPublishDiagnosticsTask(for: note.textDocument.uri)
329+
await diagnosticReportManager.removeItemsFromCache(with: note.textDocument.uri)
320330

321331
let keys = self.keys
322332

@@ -340,6 +350,7 @@ extension SwiftLanguageServer {
340350
public func closeDocument(_ note: DidCloseTextDocumentNotification) async {
341351
cancelInFlightPublishDiagnosticsTask(for: note.textDocument.uri)
342352
inFlightPublishDiagnosticsTasks[note.textDocument.uri] = nil
353+
await diagnosticReportManager.removeItemsFromCache(with: note.textDocument.uri)
343354

344355
let keys = self.keys
345356

@@ -394,8 +405,11 @@ extension SwiftLanguageServer {
394405
return
395406
}
396407
do {
397-
let diagnosticReport = try await self.fullDocumentDiagnosticReport(
398-
DocumentDiagnosticsRequest(textDocument: TextDocumentIdentifier(document))
408+
let snapshot = try await documentManager.latestSnapshot(document)
409+
let buildSettings = await self.buildSettings(for: document)
410+
let diagnosticReport = try await self.diagnosticReportManager.diagnosticReport(
411+
for: snapshot,
412+
buildSettings: buildSettings
399413
)
400414

401415
await sourceKitServer.sendNotificationToClient(
@@ -697,10 +711,11 @@ extension SwiftLanguageServer {
697711
}
698712

699713
func retrieveQuickFixCodeActions(_ params: CodeActionRequest) async throws -> [CodeAction] {
700-
let diagnosticReport = try await self.fullDocumentDiagnosticReport(
701-
DocumentDiagnosticsRequest(
702-
textDocument: params.textDocument
703-
)
714+
let snapshot = try documentManager.latestSnapshot(params.textDocument.uri)
715+
let buildSettings = await self.buildSettings(for: params.textDocument.uri)
716+
let diagnosticReport = try await self.diagnosticReportManager.diagnosticReport(
717+
for: snapshot,
718+
buildSettings: buildSettings
704719
)
705720

706721
let codeActions = diagnosticReport.items.flatMap { (diag) -> [CodeAction] in
@@ -775,24 +790,15 @@ extension SwiftLanguageServer {
775790
return Array(hints)
776791
}
777792

778-
public func syntacticDiagnosticFromBuiltInSwiftSyntax(
779-
for snapshot: DocumentSnapshot
780-
) async throws -> RelatedFullDocumentDiagnosticReport {
781-
let syntaxTree = await syntaxTreeManager.syntaxTree(for: snapshot)
782-
let swiftSyntaxDiagnostics = ParseDiagnosticsGenerator.diagnostics(for: syntaxTree)
783-
let diagnostics = swiftSyntaxDiagnostics.compactMap { (diag) -> Diagnostic? in
784-
if diag.diagnosticID == StaticTokenError.editorPlaceholder.diagnosticID {
785-
// Ignore errors about editor placeholders in the source file, similar to how sourcekitd ignores them.
786-
return nil
787-
}
788-
return Diagnostic(diag, in: snapshot)
789-
}
790-
return RelatedFullDocumentDiagnosticReport(items: diagnostics)
791-
}
792-
793793
public func documentDiagnostic(_ req: DocumentDiagnosticsRequest) async throws -> DocumentDiagnosticReport {
794+
let snapshot = try documentManager.latestSnapshot(req.textDocument.uri)
795+
let buildSettings = await self.buildSettings(for: req.textDocument.uri)
796+
let diagnosticReport = try await self.diagnosticReportManager.diagnosticReport(
797+
for: snapshot,
798+
buildSettings: buildSettings
799+
)
794800
do {
795-
return try await .full(fullDocumentDiagnosticReport(req))
801+
return try await .full(diagnosticReport)
796802
} catch {
797803
// VS Code does not request diagnostics again for a document if the diagnostics request failed.
798804
// Since sourcekit-lsp usually recovers from failures (e.g. after sourcekitd crashes), this is undesirable.
@@ -807,53 +813,6 @@ extension SwiftLanguageServer {
807813
}
808814
}
809815

810-
private func fullDocumentDiagnosticReport(
811-
_ req: DocumentDiagnosticsRequest
812-
) async throws -> RelatedFullDocumentDiagnosticReport {
813-
let snapshot = try documentManager.latestSnapshot(req.textDocument.uri)
814-
guard let buildSettings = await self.buildSettings(for: req.textDocument.uri), !buildSettings.isFallback else {
815-
logger.log(
816-
"Producing syntactic diagnostics from the built-in swift-syntax because we have fallback arguments"
817-
)
818-
// If we don't have build settings or we only have fallback build settings,
819-
// sourcekitd won't be able to give us accurate semantic diagnostics.
820-
// Fall back to providing syntactic diagnostics from the built-in
821-
// swift-syntax. That's the best we can do for now.
822-
return try await syntacticDiagnosticFromBuiltInSwiftSyntax(for: snapshot)
823-
}
824-
825-
try Task.checkCancellation()
826-
827-
let keys = self.keys
828-
829-
let skreq = sourcekitd.dictionary([
830-
keys.request: requests.diagnostics,
831-
keys.sourcefile: snapshot.uri.pseudoPath,
832-
keys.compilerargs: buildSettings.compilerArgs as [SKDValue],
833-
])
834-
835-
let dict = try await self.sourcekitd.send(skreq, fileContents: snapshot.text)
836-
837-
try Task.checkCancellation()
838-
guard (try? documentManager.latestSnapshot(req.textDocument.uri).id) == snapshot.id else {
839-
// Check that the document wasn't modified while we were getting diagnostics. This could happen because we are
840-
// calling `fullDocumentDiagnosticReport` from `publishDiagnosticsIfNeeded` outside of `messageHandlingQueue`
841-
// and thus a concurrent edit is possible while we are waiting for the sourcekitd request to return a result.
842-
throw ResponseError.unknown("Document was modified while loading document")
843-
}
844-
845-
let supportsCodeDescription = capabilityRegistry.clientHasDiagnosticsCodeDescriptionSupport
846-
var diagnostics: [Diagnostic] = []
847-
dict[keys.diagnostics]?.forEach { _, diag in
848-
if let diag = Diagnostic(diag, in: snapshot, useEducationalNoteAsCode: supportsCodeDescription) {
849-
diagnostics.append(diag)
850-
}
851-
return true
852-
}
853-
854-
return RelatedFullDocumentDiagnosticReport(items: diagnostics)
855-
}
856-
857816
public func executeCommand(_ req: ExecuteCommandRequest) async throws -> LSPAny? {
858817
// TODO: If there's support for several types of commands, we might need to structure this similarly to the code actions request.
859818
guard let sourceKitServer else {

0 commit comments

Comments
 (0)