Skip to content

Commit 3e432c4

Browse files
authored
Merge pull request #965 from joehsieh/cache-diagnostics
2 parents 403fc5e + 8dcfb30 commit 3e432c4

File tree

4 files changed

+215
-75
lines changed

4 files changed

+215
-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: 180 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,180 @@
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+
let diagnostics: [Diagnostic] =
121+
dict[keys.diagnostics]?.compactMap({ diag in
122+
Diagnostic(
123+
diag,
124+
in: snapshot,
125+
useEducationalNoteAsCode: self.clientHasDiagnosticsCodeDescriptionSupport
126+
)
127+
}) ?? []
128+
129+
return RelatedFullDocumentDiagnosticReport(items: diagnostics)
130+
}
131+
132+
private func requestFallbackReport(
133+
with snapshot: DocumentSnapshot
134+
) async throws -> LanguageServerProtocol.RelatedFullDocumentDiagnosticReport {
135+
// If we don't have build settings or we only have fallback build settings,
136+
// sourcekitd won't be able to give us accurate semantic diagnostics.
137+
// Fall back to providing syntactic diagnostics from the built-in
138+
// swift-syntax. That's the best we can do for now.
139+
let syntaxTree = await syntaxTreeManager.syntaxTree(for: snapshot)
140+
let swiftSyntaxDiagnostics = ParseDiagnosticsGenerator.diagnostics(for: syntaxTree)
141+
let diagnostics = swiftSyntaxDiagnostics.compactMap { (diag) -> Diagnostic? in
142+
if diag.diagnosticID == StaticTokenError.editorPlaceholder.diagnosticID {
143+
// Ignore errors about editor placeholders in the source file, similar to how sourcekitd ignores them.
144+
return nil
145+
}
146+
return Diagnostic(diag, in: snapshot)
147+
}
148+
return RelatedFullDocumentDiagnosticReport(items: diagnostics)
149+
}
150+
151+
/// The reportTask for the given document snapshot and buildSettings.
152+
private func reportTask(
153+
for snapshotID: DocumentSnapshot.ID,
154+
buildSettings: SwiftCompileCommand?
155+
) -> ReportTask? {
156+
return reportTaskCache.first(where: { $0.snapshotID == snapshotID && $0.buildSettings == buildSettings })?
157+
.reportTask
158+
}
159+
160+
/// Set the reportTask for the given document snapshot and buildSettings.
161+
///
162+
/// If we are already storing `cacheSize` many reports, the oldest one
163+
/// will get discarded.
164+
private func setReportTask(
165+
for snapshotID: DocumentSnapshot.ID,
166+
buildSettings: SwiftCompileCommand?,
167+
reportTask: ReportTask
168+
) {
169+
reportTaskCache.insert((snapshotID, buildSettings, reportTask), at: 0)
170+
171+
// Remove any reportTasks for old versions of this document.
172+
reportTaskCache.removeAll(where: { $0.snapshotID < snapshotID })
173+
174+
// If we still have more than `cacheSize` reportTasks, delete the ones that
175+
// were produced last. We can always re-request them on-demand.
176+
while reportTaskCache.count > cacheSize {
177+
reportTaskCache.removeLast()
178+
}
179+
}
180+
}

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(
@@ -690,10 +704,11 @@ extension SwiftLanguageServer {
690704
}
691705

692706
func retrieveQuickFixCodeActions(_ params: CodeActionRequest) async throws -> [CodeAction] {
693-
let diagnosticReport = try await self.fullDocumentDiagnosticReport(
694-
DocumentDiagnosticsRequest(
695-
textDocument: params.textDocument
696-
)
707+
let snapshot = try documentManager.latestSnapshot(params.textDocument.uri)
708+
let buildSettings = await self.buildSettings(for: params.textDocument.uri)
709+
let diagnosticReport = try await self.diagnosticReportManager.diagnosticReport(
710+
for: snapshot,
711+
buildSettings: buildSettings
697712
)
698713

699714
let codeActions = diagnosticReport.items.flatMap { (diag) -> [CodeAction] in
@@ -768,24 +783,15 @@ extension SwiftLanguageServer {
768783
return Array(hints)
769784
}
770785

771-
public func syntacticDiagnosticFromBuiltInSwiftSyntax(
772-
for snapshot: DocumentSnapshot
773-
) async throws -> RelatedFullDocumentDiagnosticReport {
774-
let syntaxTree = await syntaxTreeManager.syntaxTree(for: snapshot)
775-
let swiftSyntaxDiagnostics = ParseDiagnosticsGenerator.diagnostics(for: syntaxTree)
776-
let diagnostics = swiftSyntaxDiagnostics.compactMap { (diag) -> Diagnostic? in
777-
if diag.diagnosticID == StaticTokenError.editorPlaceholder.diagnosticID {
778-
// Ignore errors about editor placeholders in the source file, similar to how sourcekitd ignores them.
779-
return nil
780-
}
781-
return Diagnostic(diag, in: snapshot)
782-
}
783-
return RelatedFullDocumentDiagnosticReport(items: diagnostics)
784-
}
785-
786786
public func documentDiagnostic(_ req: DocumentDiagnosticsRequest) async throws -> DocumentDiagnosticReport {
787+
let snapshot = try documentManager.latestSnapshot(req.textDocument.uri)
788+
let buildSettings = await self.buildSettings(for: req.textDocument.uri)
789+
let diagnosticReport = try await self.diagnosticReportManager.diagnosticReport(
790+
for: snapshot,
791+
buildSettings: buildSettings
792+
)
787793
do {
788-
return try await .full(fullDocumentDiagnosticReport(req))
794+
return try await .full(diagnosticReport)
789795
} catch {
790796
// VS Code does not request diagnostics again for a document if the diagnostics request failed.
791797
// Since sourcekit-lsp usually recovers from failures (e.g. after sourcekitd crashes), this is undesirable.
@@ -800,53 +806,6 @@ extension SwiftLanguageServer {
800806
}
801807
}
802808

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

0 commit comments

Comments
 (0)