Skip to content

Commit 2aec5d4

Browse files
committed
[CodeCompletionn] Block completions in other threads
if fast-completion is enabled. So they have higher chance to use the cached completion instance. If it's disabled, don't block, use an ephemeral instance so we can peform multiple completions simultaneously.
1 parent fe07d44 commit 2aec5d4

File tree

3 files changed

+28
-22
lines changed

3 files changed

+28
-22
lines changed

include/swift/IDE/CompletionInstance.h

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,11 @@ class CompletionInstance {
4040
unsigned MaxASTReuseCount = 100;
4141
bool EnableASTCaching = false;
4242

43-
struct CachedInstance {
44-
CompilerInstance CI;
45-
llvm::hash_code ArgHash;
46-
unsigned ReuseCound = 0;
47-
};
48-
std::shared_ptr<CachedInstance> CachedInst;
43+
std::mutex mtx;
44+
45+
std::unique_ptr<CompilerInstance> CachedCI;
46+
llvm::hash_code CachedArgHash;
47+
unsigned CachedReuseCound = 0;
4948

5049
/// Calls \p Callback with cached \c CompilerInstance if it's usable for the
5150
/// specified completion request.

lib/IDE/CompletionInstance.cpp

Lines changed: 19 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -138,20 +138,17 @@ bool CompletionInstance::performCachedOperaitonIfPossible(
138138
if (!EnableASTCaching)
139139
return false;
140140

141-
// Temporary move the CI so other threads don't use the same instance.
142-
std::shared_ptr<CachedInstance> CachedI(nullptr);
143-
CachedInst.swap(CachedI);
144-
145-
if (!CachedI)
141+
if (!CachedCI)
146142
return false;
147-
if (CachedI->ReuseCound >= MaxASTReuseCount)
143+
144+
if (CachedReuseCound >= MaxASTReuseCount)
148145
return false;
149-
if (CachedI->ArgHash != ArgsHash)
146+
if (CachedArgHash != ArgsHash)
150147
return false;
151148

152-
auto &CI = CachedI->CI;
149+
auto &CI = *CachedCI;
153150

154-
auto &oldState = CachedI->CI.getPersistentParserState();
151+
auto &oldState = CI.getPersistentParserState();
155152
if (!oldState.hasCodeCompletionDelayedDeclState())
156153
return false;
157154

@@ -242,8 +239,7 @@ bool CompletionInstance::performCachedOperaitonIfPossible(
242239
if (DiagC)
243240
CI.removeDiagnosticConsumer(DiagC);
244241

245-
CachedI->ReuseCound += 1;
246-
CachedInst.swap(CachedI);
242+
CachedReuseCound += 1;
247243

248244
return true;
249245
}
@@ -254,9 +250,9 @@ bool CompletionInstance::performNewOperation(
254250
llvm::MemoryBuffer *completionBuffer, unsigned int Offset,
255251
std::string &Error, DiagnosticConsumer *DiagC,
256252
llvm::function_ref<void(CompilerInstance &)> Callback) {
257-
CachedInst.reset();
258-
auto TheInstance = std::make_shared<CachedInstance>();
259-
auto &CI = TheInstance->CI;
253+
254+
auto TheInstance = std::make_unique<CompilerInstance>();
255+
auto &CI = *TheInstance;
260256
if (DiagC)
261257
CI.addDiagnosticConsumer(DiagC);
262258

@@ -278,8 +274,9 @@ bool CompletionInstance::performNewOperation(
278274
CI.removeDiagnosticConsumer(DiagC);
279275

280276
if (EnableASTCaching) {
281-
TheInstance->ArgHash = ArgsHash;
282-
CachedInst.swap(TheInstance);
277+
CachedCI = std::move(TheInstance);
278+
CachedArgHash = ArgsHash;
279+
CachedReuseCound = 0;
283280
}
284281

285282
return true;
@@ -308,6 +305,12 @@ bool swift::ide::CompletionInstance::performOperation(
308305
for (auto arg : Args)
309306
ArgsHash = llvm::hash_combine(ArgsHash, StringRef(arg));
310307

308+
// If AST caching is enabled, block completions in other threads. So they
309+
// have higher chance to use the cached completion instance.
310+
Optional<std::lock_guard<std::mutex>> lock;
311+
if (EnableASTCaching)
312+
lock.emplace(mtx);
313+
311314
if (performCachedOperaitonIfPossible(Invocation, ArgsHash, completionBuffer,
312315
Offset, DiagC, Callback))
313316
return true;

test/SourceKit/CodeComplete/complete_sequence_race.swift

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,12 @@ func bar(arg: Bar) {
2323
// RUN: -req=complete -pos=15:11 %s -async -- %s == \
2424
// RUN: -req=complete -pos=12:11 %s -async -- %s == \
2525
// RUN: -req=complete -pos=15:11 %s -async -- %s == \
26+
// RUN: -req=complete -pos=17:1 %s -async -- %s == \
2627
// RUN: -req=complete -pos=12:11 %s -async -- %s == \
2728
// RUN: -req=complete -pos=15:11 %s -async -- %s == \
2829
// RUN: -req=complete -pos=12:11 %s -async -- %s == \
2930
// RUN: -req=complete -pos=15:11 %s -async -- %s == \
31+
// RUN: -req=complete -pos=17:1 %s -async -- %s == \
3032
// RUN: -req=complete -pos=12:11 %s -async -- %s == \
3133
// RUN: -req=complete -pos=15:11 %s -async -- %s
3234

@@ -36,9 +38,11 @@ func bar(arg: Bar) {
3638
// RUN: -req=complete -req-opts=reuseastcontext=1 -pos=15:11 %s -async -- %s == \
3739
// RUN: -req=complete -req-opts=reuseastcontext=1 -pos=12:11 %s -async -- %s == \
3840
// RUN: -req=complete -req-opts=reuseastcontext=1 -pos=15:11 %s -async -- %s == \
41+
// RUN: -req=complete -req-opts=reuseastcontext=1 -pos=17:1 %s -async -- %s == \
3942
// RUN: -req=complete -req-opts=reuseastcontext=1 -pos=12:11 %s -async -- %s == \
4043
// RUN: -req=complete -req-opts=reuseastcontext=1 -pos=15:11 %s -async -- %s == \
4144
// RUN: -req=complete -req-opts=reuseastcontext=1 -pos=12:11 %s -async -- %s == \
4245
// RUN: -req=complete -req-opts=reuseastcontext=1 -pos=15:11 %s -async -- %s == \
46+
// RUN: -req=complete -req-opts=reuseastcontext=1 -pos=17:1 %s -async -- %s == \
4347
// RUN: -req=complete -req-opts=reuseastcontext=1 -pos=12:11 %s -async -- %s == \
4448
// RUN: -req=complete -req-opts=reuseastcontext=1 -pos=15:11 %s -async -- %s

0 commit comments

Comments
 (0)