Skip to content

Commit 2a0902b

Browse files
committed
Cache container names in CheckedIndex
It is important that we cache this because we might find a lot of symbols in the same container for eg. workspace symbols (eg. consider many symbols in the same C++ namespace). If we didn't cache this value, then we would need to perform a `primaryDefinitionOrDeclarationOccurrence` lookup for all of these containers, which is expensive. For example, searching for `only` in SourceKit-LSP’s codebase used to not return results in more than 20s (after which I gave up) and now returns >8000 results in <2s. rdar://141412138
1 parent f653ef3 commit 2a0902b

File tree

2 files changed

+90
-38
lines changed

2 files changed

+90
-38
lines changed

Sources/SemanticIndex/CheckedIndex.swift

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,33 @@ package final class CheckedIndex {
6060
private var checker: IndexOutOfDateChecker
6161
private let index: IndexStoreDB
6262

63+
/// Maps the USR of a symbol’s container the name of that container as well as the name of all containers the
64+
/// container might itself be contained in.
65+
///
66+
/// It is important that we cache this because we might find a lot of symbols in the same container for eg. workspace
67+
/// symbols (eg. consider many symbols in the same C++ namespace). If we didn't cache this value, then we would need
68+
/// to perform a `primaryDefinitionOrDeclarationOccurrence` lookup for all of these containers, which is expensive.
69+
///
70+
/// Since we don't expect `CheckedIndex` to be outlive a single request it is acceptable to cache these results
71+
/// without having any invalidation logic (similar to how we don't invalide results cached in
72+
/// `IndexOutOfDateChecker`).
73+
///
74+
/// ### Examples
75+
/// If we have
76+
/// ```swift
77+
/// struct Foo {}
78+
/// ``` then
79+
/// `containerNamesCache[<usr of Foo>]` will be `["Foo"]`.
80+
///
81+
/// If we have
82+
/// ```swift
83+
/// struct Bar {
84+
/// struct Foo {}
85+
/// }
86+
/// ```, then
87+
/// `containerNamesCache[<usr of Foo>]` will be `["Bar", "Foo"]`.
88+
private var containerNamesCache: [String: [String]] = [:]
89+
6390
fileprivate init(index: IndexStoreDB, checkLevel: IndexCheckLevel) {
6491
self.index = index
6592
self.checker = IndexOutOfDateChecker(checkLevel: checkLevel)
@@ -183,6 +210,68 @@ package final class CheckedIndex {
183210
}
184211
return result
185212
}
213+
214+
/// The names of all containers the symbol is contained in, from outermost to innermost.
215+
///
216+
/// ### Examples
217+
/// In the following, the container names of `test` are `["Foo"]`.
218+
/// ```swift
219+
/// struct Foo {
220+
/// func test() {}
221+
/// }
222+
/// ```
223+
///
224+
/// In the following, the container names of `test` are `["Bar", "Foo"]`.
225+
/// ```swift
226+
/// struct Bar {
227+
/// struct Foo {
228+
/// func test() {}
229+
/// }
230+
/// }
231+
/// ```
232+
package func containerNames(of symbol: SymbolOccurrence) -> [String] {
233+
// The container name of accessors is the container of the surrounding variable.
234+
let accessorOf = symbol.relations.filter { $0.roles.contains(.accessorOf) }
235+
if let primaryVariable = accessorOf.sorted().first {
236+
if accessorOf.count > 1 {
237+
logger.fault("Expected an occurrence to an accessor of at most one symbol, not multiple")
238+
}
239+
if let primaryVariable = primaryDefinitionOrDeclarationOccurrence(ofUSR: primaryVariable.symbol.usr) {
240+
return containerNames(of: primaryVariable)
241+
}
242+
}
243+
244+
let containers = symbol.relations.filter { $0.roles.contains(.childOf) }
245+
if containers.count > 1 {
246+
logger.fault("Expected an occurrence to a child of at most one symbol, not multiple")
247+
}
248+
let container = containers.filter {
249+
switch $0.symbol.kind {
250+
case .module, .namespace, .enum, .struct, .class, .protocol, .extension, .union:
251+
return true
252+
case .unknown, .namespaceAlias, .macro, .typealias, .function, .variable, .field, .enumConstant,
253+
.instanceMethod, .classMethod, .staticMethod, .instanceProperty, .classProperty, .staticProperty, .constructor,
254+
.destructor, .conversionFunction, .parameter, .using, .concept, .commentTag:
255+
return false
256+
}
257+
}.sorted().first
258+
259+
if let container {
260+
if let cached = containerNamesCache[container.symbol.usr] {
261+
return cached
262+
}
263+
let result: [String]
264+
if let containerDefinition = primaryDefinitionOrDeclarationOccurrence(ofUSR: container.symbol.usr) {
265+
result = self.containerNames(of: containerDefinition) + [container.symbol.name]
266+
} else {
267+
result = [container.symbol.name]
268+
}
269+
containerNamesCache[container.symbol.usr] = result
270+
return result
271+
} else {
272+
return []
273+
}
274+
}
186275
}
187276

188277
/// A wrapper around `IndexStoreDB` that allows the retrieval of a `CheckedIndex` with a specified check level or the

Sources/SourceKitLSP/SourceKitLSPServer.swift

Lines changed: 1 addition & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -2395,48 +2395,11 @@ private let maxWorkspaceSymbolResults = 4096
23952395
package typealias Diagnostic = LanguageServerProtocol.Diagnostic
23962396

23972397
fileprivate extension CheckedIndex {
2398-
func containerNames(of symbol: SymbolOccurrence) -> [String] {
2399-
// The container name of accessors is the container of the surrounding variable.
2400-
let accessorOf = symbol.relations.filter { $0.roles.contains(.accessorOf) }
2401-
if let primaryVariable = accessorOf.sorted().first {
2402-
if accessorOf.count > 1 {
2403-
logger.fault("Expected an occurrence to an accessor of at most one symbol, not multiple")
2404-
}
2405-
if let primaryVariable = primaryDefinitionOrDeclarationOccurrence(ofUSR: primaryVariable.symbol.usr) {
2406-
return containerNames(of: primaryVariable)
2407-
}
2408-
}
2409-
2410-
let containers = symbol.relations.filter { $0.roles.contains(.childOf) }
2411-
if containers.count > 1 {
2412-
logger.fault("Expected an occurrence to a child of at most one symbol, not multiple")
2413-
}
2414-
let container = containers.filter {
2415-
switch $0.symbol.kind {
2416-
case .module, .namespace, .enum, .struct, .class, .protocol, .extension, .union:
2417-
return true
2418-
case .unknown, .namespaceAlias, .macro, .typealias, .function, .variable, .field, .enumConstant,
2419-
.instanceMethod, .classMethod, .staticMethod, .instanceProperty, .classProperty, .staticProperty, .constructor,
2420-
.destructor, .conversionFunction, .parameter, .using, .concept, .commentTag:
2421-
return false
2422-
}
2423-
}.sorted().first
2424-
2425-
if let container {
2426-
if let containerDefinition = primaryDefinitionOrDeclarationOccurrence(ofUSR: container.symbol.usr) {
2427-
return self.containerNames(of: containerDefinition) + [container.symbol.name]
2428-
}
2429-
return [container.symbol.name]
2430-
} else {
2431-
return []
2432-
}
2433-
}
2434-
24352398
/// Take the name of containers into account to form a fully-qualified name for the given symbol.
24362399
/// This means that we will form names of nested types and type-qualify methods.
24372400
func fullyQualifiedName(of symbolOccurrence: SymbolOccurrence) -> String {
24382401
let symbol = symbolOccurrence.symbol
2439-
let containerNames = containerNames(of: symbolOccurrence)
2402+
let containerNames = self.containerNames(of: symbolOccurrence)
24402403
guard let containerName = containerNames.last else {
24412404
// No containers, so nothing to do.
24422405
return symbol.name

0 commit comments

Comments
 (0)