Skip to content

Commit af5daed

Browse files
committed
[CodeCompletion] Tweak fast completion dependency checking
- Detect same file with bufferID instead of the file name - Compare virtual in-memory filesystem content with hash value
1 parent e32de78 commit af5daed

File tree

5 files changed

+127
-59
lines changed

5 files changed

+127
-59
lines changed

include/swift/IDE/CompletionInstance.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,10 @@ class CompletionInstance {
4444
std::mutex mtx;
4545

4646
std::unique_ptr<CompilerInstance> CachedCI;
47+
ModuleDecl *CurrentModule = nullptr;
4748
llvm::hash_code CachedArgHash;
4849
llvm::sys::TimePoint<> DependencyCheckedTimestamp;
50+
llvm::StringMap<llvm::hash_code> InMemoryDependencyHash;
4951
unsigned CachedReuseCount = 0;
5052

5153
void cacheCompilerInstance(std::unique_ptr<CompilerInstance> CI,

lib/IDE/CompletionInstance.cpp

Lines changed: 104 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -18,19 +18,19 @@
1818
#include "swift/AST/Module.h"
1919
#include "swift/AST/PrettyStackTrace.h"
2020
#include "swift/AST/SourceFile.h"
21-
#include "swift/Serialization/SerializedModuleLoader.h"
22-
#include "swift/ClangImporter/ClangModule.h"
2321
#include "swift/Basic/LangOptions.h"
2422
#include "swift/Basic/PrettyStackTrace.h"
2523
#include "swift/Basic/SourceManager.h"
24+
#include "swift/ClangImporter/ClangModule.h"
2625
#include "swift/Driver/FrontendUtil.h"
2726
#include "swift/Frontend/Frontend.h"
2827
#include "swift/Parse/Lexer.h"
2928
#include "swift/Parse/PersistentParserState.h"
29+
#include "swift/Serialization/SerializedModuleLoader.h"
3030
#include "swift/Subsystems.h"
31+
#include "clang/AST/ASTContext.h"
3132
#include "llvm/ADT/Hashing.h"
3233
#include "llvm/Support/MemoryBuffer.h"
33-
#include "clang/AST/ASTContext.h"
3434

3535
using namespace swift;
3636
using namespace ide;
@@ -165,72 +165,113 @@ static DeclContext *getEquivalentDeclContextFromSourceFile(DeclContext *DC,
165165
return newDC;
166166
}
167167

168-
/// Check if any dependent files are modified since \p timestamp.
169-
bool areAnyDependentFilesInvalidated(CompilerInstance &CI,
170-
llvm::vfs::FileSystem &FS,
171-
StringRef currentFileName,
172-
llvm::sys::TimePoint<> timestamp) {
173-
174-
auto isInvalidated = [&](StringRef filePath) -> bool {
175-
auto stat = FS.status(filePath);
176-
if (!stat)
177-
// Missing.
178-
return true;
179-
180-
auto lastModTime = stat->getLastModificationTime();
181-
if (lastModTime > timestamp)
182-
// Modified.
183-
return true;
184-
185-
// If the last modification time is zero, this file is probably from a
186-
// virtual file system. We need to check the content.
187-
if (lastModTime == llvm::sys::TimePoint<>()) {
188-
if (&CI.getFileSystem() == &FS)
189-
return false;
190-
191-
auto oldContent = CI.getFileSystem().getBufferForFile(filePath);
192-
auto newContent = FS.getBufferForFile(filePath);
193-
if (!oldContent || !newContent)
194-
// (unreachable?)
195-
return true;
196-
197-
if (oldContent.get()->getBuffer() != newContent.get()->getBuffer())
198-
// Different content.
199-
return true;
200-
}
201-
202-
return false;
203-
};
204-
168+
/// For each dependency file in \p CI, run \p callback until the callback
169+
/// returns \c true. Returns \c true if any callback call returns \c true, \c
170+
/// false otherwise.
171+
static bool
172+
forEachDependencyUntilTrue(CompilerInstance &CI, ModuleDecl *CurrentModule,
173+
unsigned excludeBufferID,
174+
llvm::function_ref<bool(StringRef)> callback) {
205175
// Check files in the current module.
206-
for (FileUnit *file : CI.getMainModule()->getFiles()) {
176+
for (FileUnit *file : CurrentModule->getFiles()) {
207177
StringRef filename;
208-
if (auto SF = dyn_cast<SourceFile>(file))
178+
if (auto SF = dyn_cast<SourceFile>(file)) {
179+
if (SF->getBufferID() == excludeBufferID)
180+
continue;
209181
filename = SF->getFilename();
210-
else if (auto LF = dyn_cast<LoadedFile>(file))
182+
} else if (auto LF = dyn_cast<LoadedFile>(file))
211183
filename = LF->getFilename();
212184
else
213185
continue;
214186

215-
// Ignore the current file and synthesized files.
216-
if (filename.empty() || filename.front() == '<' ||
217-
filename.equals(currentFileName))
187+
// Ignore synthesized files.
188+
if (filename.empty() || filename.front() == '<')
218189
continue;
219190

220-
if (isInvalidated(filename))
191+
if (callback(filename))
221192
return true;
222193
}
223194

224195
// Check other non-system depenencies (e.g. modules, headers).
225196
for (auto &dep : CI.getDependencyTracker()->getDependencies()) {
226-
if (isInvalidated(dep))
197+
if (callback(dep))
227198
return true;
228199
}
229200

230-
// All loaded module files are not modified since the timestamp.
231201
return false;
232202
}
233203

204+
/// Collect hash codes of the dependencies into \c Map.
205+
static void cacheDependencyHashIfNeeded(CompilerInstance &CI,
206+
ModuleDecl *CurrentModule,
207+
unsigned excludeBufferID,
208+
llvm::StringMap<llvm::hash_code> &Map) {
209+
auto &FS = CI.getFileSystem();
210+
forEachDependencyUntilTrue(
211+
CI, CurrentModule, excludeBufferID, [&](StringRef filename) {
212+
if (Map.count(filename))
213+
return false;
214+
215+
auto stat = FS.status(filename);
216+
if (!stat)
217+
return false;
218+
219+
// We will check the hash only if the modification time of the dependecy
220+
// is zero. See 'areAnyDependentFilesInvalidated() below'.
221+
if (stat->getLastModificationTime() != llvm::sys::TimePoint<>())
222+
return false;
223+
224+
auto buf = FS.getBufferForFile(filename);
225+
Map[filename] = llvm::hash_value(buf.get()->getBuffer());
226+
return false;
227+
});
228+
}
229+
230+
/// Check if any dependent files are modified since \p timestamp.
231+
static bool areAnyDependentFilesInvalidated(
232+
CompilerInstance &CI, ModuleDecl *CurrentModule, llvm::vfs::FileSystem &FS,
233+
unsigned excludeBufferID, llvm::sys::TimePoint<> timestamp,
234+
llvm::StringMap<llvm::hash_code> &Map) {
235+
236+
return forEachDependencyUntilTrue(
237+
CI, CurrentModule, excludeBufferID, [&](StringRef filePath) {
238+
auto stat = FS.status(filePath);
239+
if (!stat)
240+
// Missing.
241+
return true;
242+
243+
auto lastModTime = stat->getLastModificationTime();
244+
if (lastModTime > timestamp)
245+
// Modified.
246+
return true;
247+
248+
// If the last modification time is zero, this file is probably from a
249+
// virtual file system. We need to check the content.
250+
if (lastModTime == llvm::sys::TimePoint<>()) {
251+
// Get the hash code of the last content.
252+
auto oldHashEntry = Map.find(filePath);
253+
if (oldHashEntry == Map.end())
254+
// Unreachable? Not virtual in old filesystem, but virtual in new
255+
// one.
256+
return true;
257+
auto oldHash = oldHashEntry->second;
258+
259+
// Calculate the hash code of the current content.
260+
auto newContent = FS.getBufferForFile(filePath);
261+
if (!newContent)
262+
// Unreachable? stat succeeded, but coundn't get the content.
263+
return true;
264+
265+
auto newHash = llvm::hash_value(newContent.get()->getBuffer());
266+
267+
if (oldHash != newHash)
268+
return true;
269+
}
270+
271+
return false;
272+
});
273+
}
274+
234275
} // namespace
235276

236277
bool CompletionInstance::performCachedOperationIfPossible(
@@ -262,8 +303,9 @@ bool CompletionInstance::performCachedOperationIfPossible(
262303
return false;
263304

264305
if (shouldCheckDependencies()) {
265-
if (areAnyDependentFilesInvalidated(CI, *FileSystem, bufferName,
266-
DependencyCheckedTimestamp))
306+
if (areAnyDependentFilesInvalidated(
307+
CI, CurrentModule, *FileSystem, SM.getCodeCompletionBufferID(),
308+
DependencyCheckedTimestamp, InMemoryDependencyHash))
267309
return false;
268310
DependencyCheckedTimestamp = std::chrono::system_clock::now();
269311
}
@@ -406,6 +448,7 @@ bool CompletionInstance::performCachedOperationIfPossible(
406448
performImportResolution(*newSF);
407449
bindExtensions(*newSF);
408450

451+
CurrentModule = newM;
409452
traceDC = newM;
410453
#ifndef NDEBUG
411454
const auto *reparsedState = newSF->getDelayedParserState();
@@ -432,6 +475,8 @@ bool CompletionInstance::performCachedOperationIfPossible(
432475
}
433476

434477
CachedReuseCount += 1;
478+
cacheDependencyHashIfNeeded(CI, CurrentModule, SM.getCodeCompletionBufferID(),
479+
InMemoryDependencyHash);
435480

436481
return true;
437482
}
@@ -500,17 +545,24 @@ bool CompletionInstance::performNewOperation(
500545
void CompletionInstance::cacheCompilerInstance(
501546
std::unique_ptr<CompilerInstance> CI, llvm::hash_code ArgsHash) {
502547
CachedCI = std::move(CI);
548+
CurrentModule = CachedCI->getMainModule();
503549
CachedArgHash = ArgsHash;
504550
auto now = std::chrono::system_clock::now();
505551
DependencyCheckedTimestamp = now;
506552
CachedReuseCount = 0;
553+
InMemoryDependencyHash.clear();
554+
cacheDependencyHashIfNeeded(
555+
*CachedCI, CurrentModule,
556+
CachedCI->getASTContext().SourceMgr.getCodeCompletionBufferID(),
557+
InMemoryDependencyHash);
507558
}
508559

509560
bool CompletionInstance::shouldCheckDependencies() const {
510561
assert(CachedCI);
511562
using namespace std::chrono;
512563
auto now = system_clock::now();
513-
return DependencyCheckedTimestamp + seconds(DependencyCheckIntervalSecond) < now;
564+
return DependencyCheckedTimestamp + seconds(DependencyCheckIntervalSecond) <
565+
now;
514566
}
515567

516568
void CompletionInstance::setDependencyCheckIntervalSecond(unsigned Value) {
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
extension MyStruct {
2+
func extensionMethod() {}
3+
}

test/SourceKit/CodeComplete/complete_checkdeps_vfs.swift

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,40 +8,45 @@ func foo(value: MyStruct) {
88
// RUN: SLEEP_TIME=2
99

1010
// RUN: %empty-directory(%t)
11+
// RUN: %empty-directory(%t/VFS)
12+
// RUN: cp %S/Inputs/checkdeps/MyProject/LibraryExt.swift %t/VFS/
1113

1214
// RUN: %sourcekitd-test \
1315
// RUN: -req=global-config -completion-check-dependency-interval ${DEPCHECK_INTERVAL} == \
1416

1517
// RUN: -shell -- echo "### Initial" == \
16-
// RUN: -req=complete -pos=2:9 -pass-as-sourcetext -vfs-files=%t/VFS/Main.swift=@%s,%t/VFS/Library.swift=@%S/Inputs/checkdeps/MyProject/Library.swift %t/VFS/Main.swift -- -target %target-triple %t/VFS/Main.swift %t/VFS/Library.swift == \
18+
// RUN: -req=complete -pos=2:9 -pass-as-sourcetext -vfs-files=%t/VFS/Main.swift=@%s,%t/VFS/Library.swift=@%S/Inputs/checkdeps/MyProject/Library.swift %t/VFS/Main.swift -- -target %target-triple %t/VFS/Main.swift %t/VFS/LibraryExt.swift %t/VFS/Library.swift == \
1719

1820
// RUN: -shell -- echo "### Modify" == \
1921
// RUN: -shell -- sleep ${SLEEP_TIME} == \
20-
// RUN: -req=complete -pos=2:9 -pass-as-sourcetext -vfs-files=%t/VFS/Main.swift=@%s,%t/VFS/Library.swift=@%S/Inputs/checkdeps/MyProject_mod/Library.swift %t/VFS/Main.swift -- -target %target-triple %t/VFS/Main.swift %t/VFS/Library.swift == \
22+
// RUN: -req=complete -pos=2:9 -pass-as-sourcetext -vfs-files=%t/VFS/Main.swift=@%s,%t/VFS/Library.swift=@%S/Inputs/checkdeps/MyProject_mod/Library.swift %t/VFS/Main.swift -- -target %target-triple %t/VFS/Main.swift %t/VFS/LibraryExt.swift %t/VFS/Library.swift == \
2123

2224
// RUN: -shell -- echo "### Keep" == \
2325
// RUN: -shell -- sleep ${SLEEP_TIME} == \
24-
// RUN: -req=complete -pos=2:9 -pass-as-sourcetext -vfs-files=%t/VFS/Main.swift=@%s,%t/VFS/Library.swift=@%S/Inputs/checkdeps/MyProject_mod/Library.swift %t/VFS/Main.swift -- -target %target-triple %t/VFS/Main.swift %t/VFS/Library.swift \
26+
// RUN: -req=complete -pos=2:9 -pass-as-sourcetext -vfs-files=%t/VFS/Main.swift=@%s,%t/VFS/Library.swift=@%S/Inputs/checkdeps/MyProject_mod/Library.swift %t/VFS/Main.swift -- -target %target-triple %t/VFS/Main.swift %t/VFS/LibraryExt.swift %t/VFS/Library.swift \
2527

2628
// RUN: | %FileCheck %s
2729

2830
// CHECK-LABEL: ### Initial
2931
// CHECK: key.results: [
3032
// CHECK-DAG: key.description: "myStructMethod()"
33+
// CHECK-DAG: key.description: "extensionMethod()"
3134
// CHECK-DAG: key.description: "self"
3235
// CHECK: ]
3336
// CHECK-NOT: key.reusingastcontext: 1
3437

3538
// CHECK-LABEL: ### Modify
3639
// CHECK: key.results: [
3740
// CHECK-DAG: key.description: "myStructMethod_mod()"
41+
// CHECK-DAG: key.description: "extensionMethod()"
3842
// CHECK-DAG: key.description: "self"
3943
// CHECK: ]
4044
// CHECK-NOT: key.reusingastcontext: 1
4145

4246
// CHECK-LABEL: ### Keep
4347
// CHECK: key.results: [
4448
// CHECK-DAG: key.description: "myStructMethod_mod()"
49+
// CHECK-DAG: key.description: "extensionMethod()"
4550
// CHECK-DAG: key.description: "self"
4651
// CHECK: ]
4752
// CHECK: key.reusingastcontext: 1

test/SourceKit/CodeComplete/complete_checkdeps_vfs_open.swift

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,41 +8,47 @@ func foo(value: MyStruct) {
88
// RUN: SLEEP_TIME=2
99

1010
// RUN: %empty-directory(%t)
11+
// RUN: %empty-directory(%t/VFS)
12+
// RUN: cp %S/Inputs/checkdeps/MyProject/LibraryExt.swift %t/VFS/
13+
1114
// RUN: %sourcekitd-test \
1215
// RUN: -req=global-config -completion-check-dependency-interval ${DEPCHECK_INTERVAL} == \
1316

1417
// RUN: -shell -- echo "### Initial" == \
15-
// RUN: -req=complete.open -pos=2:9 -pass-as-sourcetext -vfs-files=%t/VFS/Main.swift=@%s,%t/VFS/Library.swift=@%S/Inputs/checkdeps/MyProject/Library.swift %t/VFS/Main.swift -- -target %target-triple %t/VFS/Main.swift %t/VFS/Library.swift == \
18+
// RUN: -req=complete.open -pos=2:9 -pass-as-sourcetext -vfs-files=%t/VFS/Main.swift=@%s,%t/VFS/Library.swift=@%S/Inputs/checkdeps/MyProject/Library.swift %t/VFS/Main.swift -- -target %target-triple %t/VFS/Main.swift %t/VFS/LibraryExt.swift %t/VFS/Library.swift == \
1619
// RUN: -req=complete.close -pos=2:9 -name %t/VFS/Main.swift %s == \
1720

1821
// RUN: -shell -- echo "### Modify" == \
1922
// RUN: -shell -- sleep ${SLEEP_TIME} == \
20-
// RUN: -req=complete.open -pos=2:9 -pass-as-sourcetext -vfs-files=%t/VFS/Main.swift=@%s,%t/VFS/Library.swift=@%S/Inputs/checkdeps/MyProject_mod/Library.swift %t/VFS/Main.swift -- -target %target-triple %t/VFS/Main.swift %t/VFS/Library.swift == \
23+
// RUN: -req=complete.open -pos=2:9 -pass-as-sourcetext -vfs-files=%t/VFS/Main.swift=@%s,%t/VFS/Library.swift=@%S/Inputs/checkdeps/MyProject_mod/Library.swift %t/VFS/Main.swift -- -target %target-triple %t/VFS/Main.swift %t/VFS/LibraryExt.swift %t/VFS/Library.swift == \
2124
// RUN: -req=complete.close -pos=2:9 -name %t/VFS/Main.swift %s == \
2225

2326
// RUN: -shell -- echo "### Keep" == \
24-
// RUN: -req=complete.open -pos=2:9 -pass-as-sourcetext -vfs-files=%t/VFS/Main.swift=@%s,%t/VFS/Library.swift=@%S/Inputs/checkdeps/MyProject_mod/Library.swift %t/VFS/Main.swift -- -target %target-triple %t/VFS/Main.swift %t/VFS/Library.swift == \
27+
// RUN: -req=complete.open -pos=2:9 -pass-as-sourcetext -vfs-files=%t/VFS/Main.swift=@%s,%t/VFS/Library.swift=@%S/Inputs/checkdeps/MyProject_mod/Library.swift %t/VFS/Main.swift -- -target %target-triple %t/VFS/Main.swift %t/VFS/LibraryExt.swift %t/VFS/Library.swift == \
2528
// RUN: -req=complete.close -pos=2:9 -name %t/VFS/Main.swift %s \
2629

27-
// RUN: | %FileCheck %s
30+
// RUN: | tee %t/trace | %FileCheck %s
2831

2932
// CHECK-LABEL: ### Initial
3033
// CHECK: key.results: [
3134
// CHECK-DAG: key.description: "myStructMethod()"
35+
// CHECK-DAG: key.description: "extensionMethod()"
3236
// CHECK-DAG: key.description: "self"
3337
// CHECK: ]
3438
// CHECK-NOT: key.reusingastcontext: 1
3539

3640
// CHECK-LABEL: ### Modify
3741
// CHECK: key.results: [
3842
// CHECK-DAG: key.description: "myStructMethod_mod()"
43+
// CHECK-DAG: key.description: "extensionMethod()"
3944
// CHECK-DAG: key.description: "self"
4045
// CHECK: ]
4146
// CHECK-NOT: key.reusingastcontext: 1
4247

4348
// CHECK-LABEL: ### Keep
4449
// CHECK: key.results: [
4550
// CHECK-DAG: key.description: "myStructMethod_mod()"
51+
// CHECK-DAG: key.description: "extensionMethod()"
4652
// CHECK-DAG: key.description: "self"
4753
// CHECK: ]
4854
// CHECK: key.reusingastcontext: 1

0 commit comments

Comments
 (0)