Skip to content

Commit e12d4de

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. (cherry picked from commit 2aec5d4)
1 parent 174c37f commit e12d4de

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)