[SourceKit] Pass 'EnableASTCaching' flag as an argument

so that it is associated with a specific completion.

include/swift/IDE/CompletionInstance.h

@@ -38,20 +38,18 @@ makeCodeCompletionMemoryBuffer(const llvm::MemoryBuffer *origBuf,
 /// Manages \c CompilerInstance for completion like operations.
 class CompletionInstance {
   unsigned MaxASTReuseCount = 100;
-  bool EnableASTCaching = false;
 
   std::mutex mtx;
 
   std::unique_ptr<CompilerInstance> CachedCI;
   llvm::hash_code CachedArgHash;
-  unsigned CachedReuseCound = 0;
+  unsigned CachedReuseCount = 0;
 
   /// Calls \p Callback with cached \c CompilerInstance if it's usable for the
   /// specified completion request.
-  /// Returns \c if the callback was called. Returns \c false if the
+  /// Returns \c if the callback was called. Returns \c false if the compiler
-  /// functionality is disabled, compiler argument has
+  /// argument has changed, primary file is not the same, the \c Offset is not
-  /// changed, primary file is not the same, the \c Offset is not in function
+  /// in function bodies, or the interface hash of the file has changed.
-  /// bodies, or the interface hash of the file has changed.
   bool performCachedOperaitonIfPossible(
       const swift::CompilerInvocation &Invocation, llvm::hash_code ArgsHash,
       llvm::MemoryBuffer *completionBuffer, unsigned int Offset,
@@ -63,15 +61,14 @@ class CompletionInstance {
   /// the first pass.
   /// Returns \c false if it fails to setup the \c CompilerInstance.
   bool performNewOperation(
-      swift::CompilerInvocation &Invocation, llvm::hash_code ArgsHash,
+      llvm::Optional<llvm::hash_code> ArgsHash,
+      swift::CompilerInvocation &Invocation,
       llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FileSystem,
       llvm::MemoryBuffer *completionBuffer, unsigned int Offset,
       std::string &Error, DiagnosticConsumer *DiagC,
       llvm::function_ref<void(CompilerInstance &)> Callback);
 
 public:
-  void setEnableASTCaching(bool Flag) { EnableASTCaching = Flag; }
-
   /// Calls \p Callback with a \c CompilerInstance which is prepared for the
   /// second pass. \p Callback is resposible to perform the second pass on it.
   /// The \c CompilerInstance may be reused from the previous completions,
@@ -82,13 +79,12 @@ public:
   /// NOTE: \p Args is only used for checking the equaity of the invocation.
   /// Since this function assumes that it is already normalized, exact the same
   /// arguments including their order is considered as the same invocation.
-  bool
+  bool performOperation(
-  performOperation(swift::CompilerInvocation &Invocation,
+      swift::CompilerInvocation &Invocation, llvm::ArrayRef<const char *> Args,
-                   llvm::ArrayRef<const char *> Args,
+      llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FileSystem,
-                   llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FileSystem,
+      llvm::MemoryBuffer *completionBuffer, unsigned int Offset,
-                   llvm::MemoryBuffer *completionBuffer, unsigned int Offset,
+      bool EnableASTCaching, std::string &Error, DiagnosticConsumer *DiagC,
-                   std::string &Error, DiagnosticConsumer *DiagC,
+      llvm::function_ref<void(CompilerInstance &)> Callback);
-                   llvm::function_ref<void(CompilerInstance &)> Callback);
 };
 
 } // namespace ide

lib/IDE/CompletionInstance.cpp

@@ -135,13 +135,9 @@ bool CompletionInstance::performCachedOperaitonIfPossible(
     DiagnosticConsumer *DiagC,
     llvm::function_ref<void(CompilerInstance &)> Callback) {
 
-  if (!EnableASTCaching)
-    return false;
-
   if (!CachedCI)
     return false;
-
+  if (CachedReuseCount >= MaxASTReuseCount)
-  if (CachedReuseCound >= MaxASTReuseCount)
     return false;
   if (CachedArgHash != ArgsHash)
     return false;
@@ -239,13 +235,13 @@ bool CompletionInstance::performCachedOperaitonIfPossible(
   if (DiagC)
     CI.removeDiagnosticConsumer(DiagC);
 
-  CachedReuseCound += 1;
+  CachedReuseCount += 1;
 
   return true;
 }
 
 bool CompletionInstance::performNewOperation(
-    swift::CompilerInvocation &Invocation, llvm::hash_code ArgsHash,
+    Optional<llvm::hash_code> ArgsHash, swift::CompilerInvocation &Invocation,
     llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FileSystem,
     llvm::MemoryBuffer *completionBuffer, unsigned int Offset,
     std::string &Error, DiagnosticConsumer *DiagC,
@@ -273,10 +269,10 @@ bool CompletionInstance::performNewOperation(
   if (DiagC)
     CI.removeDiagnosticConsumer(DiagC);
 
-  if (EnableASTCaching) {
+  if (ArgsHash.hasValue()) {
     CachedCI = std::move(TheInstance);
-    CachedArgHash = ArgsHash;
+    CachedArgHash = *ArgsHash;
-    CachedReuseCound = 0;
+    CachedReuseCount = 0;
   }
 
   return true;
@@ -286,7 +282,7 @@ bool swift::ide::CompletionInstance::performOperation(
     swift::CompilerInvocation &Invocation, llvm::ArrayRef<const char *> Args,
     llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FileSystem,
     llvm::MemoryBuffer *completionBuffer, unsigned int Offset,
-    std::string &Error, DiagnosticConsumer *DiagC,
+    bool EnableASTCaching, std::string &Error, DiagnosticConsumer *DiagC,
     llvm::function_ref<void(CompilerInstance &)> Callback) {
 
   // Always disable source location resolutions from .swiftsourceinfo file
@@ -300,24 +296,29 @@ bool swift::ide::CompletionInstance::performOperation(
   // FIXME: ASTScopeLookup doesn't support code completion yet.
   Invocation.disableASTScopeLookup();
 
-  // Compute the signature of the invocation.
+  if (EnableASTCaching) {
-  llvm::hash_code ArgsHash(0);
+    // Compute the signature of the invocation.
-  for (auto arg : Args)
+    llvm::hash_code ArgsHash(0);
-    ArgsHash = llvm::hash_combine(ArgsHash, StringRef(arg));
+    for (auto arg : Args)
+      ArgsHash = llvm::hash_combine(ArgsHash, StringRef(arg));
 
-  // If AST caching is enabled, block completions in other threads. So they
+    // Concurrent completions will block so that they have higher chance to use
-  // have higher chance to use the cached completion instance.
+    // the cached completion instance.
-  Optional<std::lock_guard<std::mutex>> lock;
+    std::lock_guard<std::mutex> lock(mtx);
-  if (EnableASTCaching)
-    lock.emplace(mtx);
 
-  if (performCachedOperaitonIfPossible(Invocation, ArgsHash, completionBuffer,
+    if (performCachedOperaitonIfPossible(Invocation, ArgsHash, completionBuffer,
-                                       Offset, DiagC, Callback))
+                                         Offset, DiagC, Callback))
-    return true;
+      return true;
 
-  if (performNewOperation(Invocation, ArgsHash, FileSystem, completionBuffer,
+    if (performNewOperation(ArgsHash, Invocation, FileSystem, completionBuffer,
-                          Offset, Error, DiagC, Callback))
+                            Offset, Error, DiagC, Callback))
-    return true;
+      return true;
+  } else {
+    // Concurrent completions may happen in parallel when caching is disabled.
+    if (performNewOperation(None, Invocation, FileSystem, completionBuffer,
+                            Offset, Error, DiagC, Callback))
+      return true;
+  }
 
   assert(!Error.empty());
   return false;

tools/SourceKit/lib/SwiftLang/SwiftCompletion.cpp

@@ -123,9 +123,9 @@ static bool swiftCodeCompleteImpl(
     unsigned Offset, SwiftCodeCompletionConsumer &SwiftConsumer,
     ArrayRef<const char *> Args,
     llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FileSystem,
-    std::string &Error) {
+    bool EnableASTCaching, std::string &Error) {
   return Lang.performCompletionLikeOperation(
-      UnresolvedInputFile, Offset, Args, FileSystem, Error,
+      UnresolvedInputFile, Offset, Args, FileSystem, EnableASTCaching, Error,
       [&](CompilerInstance &CI) {
         // Create a factory for code completion callbacks that will feed the
         // Consumer.
@@ -155,7 +155,6 @@ void SwiftLangSupport::codeComplete(
     SourceKit::CodeCompletionConsumer &SKConsumer, ArrayRef<const char *> Args,
     Optional<VFSOptions> vfsOptions) {
 
-
   CodeCompletion::Options CCOpts;
   if (options) {
     StringRef filterText;
@@ -164,7 +163,6 @@ void SwiftLangSupport::codeComplete(
     translateCodeCompletionOptions(*options, CCOpts, filterText, resultOffset,
                                    maxResults);
   }
-  CompletionInst->setEnableASTCaching(CCOpts.reuseASTContextIfPossible);
 
   std::string error;
   // FIXME: the use of None as primary file is to match the fact we do not read
@@ -210,7 +208,8 @@ void SwiftLangSupport::codeComplete(
 
   std::string Error;
   if (!swiftCodeCompleteImpl(*this, UnresolvedInputFile, Offset, SwiftConsumer,
-                             Args, fileSystem, Error)) {
+                             Args, fileSystem,
+                             CCOpts.reuseASTContextIfPossible, Error)) {
     SKConsumer.failed(Error);
   }
 }
@@ -1080,7 +1079,8 @@ static void transformAndForwardResults(
       cargs.push_back(arg.c_str());
     std::string error;
     if (!swiftCodeCompleteImpl(lang, buffer.get(), str.size(), swiftConsumer,
-                               cargs, session->getFileSystem(), error)) {
+                               cargs, session->getFileSystem(),
+                               options.reuseASTContextIfPossible, error)) {
       consumer.failed(error);
       return;
     }
@@ -1128,7 +1128,6 @@ void SwiftLangSupport::codeCompleteOpen(
   if (options)
     translateCodeCompletionOptions(*options, CCOpts, filterText, resultOffset,
                                    maxResults);
-  CompletionInst->setEnableASTCaching(CCOpts.reuseASTContextIfPossible);
 
   std::string error;
   // FIXME: the use of None as primary file is to match the fact we do not read
@@ -1179,7 +1178,8 @@ void SwiftLangSupport::codeCompleteOpen(
 
   // Invoke completion.
   if (!swiftCodeCompleteImpl(*this, inputBuf, offset, swiftConsumer,
-                             extendedArgs, fileSystem, error)) {
+                             extendedArgs, fileSystem,
+                             CCOpts.reuseASTContextIfPossible, error)) {
     consumer.failed(error);
     return;
   }
@@ -1249,7 +1249,6 @@ void SwiftLangSupport::codeCompleteUpdate(
   if (options)
     translateCodeCompletionOptions(*options, CCOpts, filterText, resultOffset,
                                    maxResults);
-  CompletionInst->setEnableASTCaching(CCOpts.reuseASTContextIfPossible);
 
   NameToPopularityMap *nameToPopularity = nullptr;
   // This reference must outlive the uses of nameToPopularity.

tools/SourceKit/lib/SwiftLang/SwiftConformingMethodList.cpp

@@ -32,9 +32,9 @@ static bool swiftConformingMethodListImpl(
     ArrayRef<const char *> ExpectedTypeNames,
     ide::ConformingMethodListConsumer &Consumer,
     llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FileSystem,
-    std::string &Error) {
+    bool EnableASTCaching, std::string &Error) {
   return Lang.performCompletionLikeOperation(
-      UnresolvedInputFile, Offset, Args, FileSystem, Error,
+      UnresolvedInputFile, Offset, Args, FileSystem, EnableASTCaching, Error,
       [&](CompilerInstance &CI) {
         // Create a factory for code completion callbacks that will feed the
         // Consumer.
@@ -176,7 +176,7 @@ void SwiftLangSupport::getConformingMethodList(
 
   if (!swiftConformingMethodListImpl(*this, UnresolvedInputFile, Offset, Args,
                                      ExpectedTypeNames, Consumer, fileSystem,
-                                     error)) {
+                                     /*EnableASTCaching=*/false, error)) {
     SKConsumer.failed(error);
   }
 }

tools/SourceKit/lib/SwiftLang/SwiftLangSupport.cpp

@@ -957,7 +957,8 @@ bool SwiftLangSupport::performCompletionLikeOperation(
     llvm::MemoryBuffer *UnresolvedInputFile, unsigned Offset,
     ArrayRef<const char *> Args,
     llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FileSystem,
-    std::string &Error, llvm::function_ref<void(CompilerInstance &)> Callback) {
+    bool EnableASTCaching, std::string &Error,
+    llvm::function_ref<void(CompilerInstance &)> Callback) {
   assert(FileSystem);
 
   // Resolve symlinks for the input file; we resolve them for the input files
@@ -1011,7 +1012,8 @@ bool SwiftLangSupport::performCompletionLikeOperation(
   auto CompletionInst = getCompletionInstance();
 
   return CompletionInst->performOperation(Invocation, Args, FileSystem,
-                                          newBuffer.get(), Offset, Error,
+                                          newBuffer.get(), Offset,
+                                          EnableASTCaching, Error,
                                           &CIDiags, Callback);
 }
 

tools/SourceKit/lib/SwiftLang/SwiftLangSupport.h

@@ -443,7 +443,7 @@ public:
       llvm::MemoryBuffer *UnresolvedInputFile, unsigned Offset,
       ArrayRef<const char *> Args,
       llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FileSystem,
-      std::string &Error,
+      bool EnableASTCaching, std::string &Error,
       llvm::function_ref<void(swift::CompilerInstance &)> Callback);
 
   //==========================================================================//

tools/SourceKit/lib/SwiftLang/SwiftTypeContextInfo.cpp

@@ -30,9 +30,9 @@ static bool swiftTypeContextInfoImpl(
     unsigned Offset, ide::TypeContextInfoConsumer &Consumer,
     ArrayRef<const char *> Args,
     llvm::IntrusiveRefCntPtr<llvm::vfs::FileSystem> FileSystem,
-    std::string &Error) {
+    bool EnableASTCaching, std::string &Error) {
   return Lang.performCompletionLikeOperation(
-      UnresolvedInputFile, Offset, Args, FileSystem, Error,
+      UnresolvedInputFile, Offset, Args, FileSystem, EnableASTCaching, Error,
       [&](CompilerInstance &CI) {
         // Create a factory for code completion callbacks that will feed the
         // Consumer.
@@ -151,7 +151,8 @@ void SwiftLangSupport::getExpressionContextInfo(
   } Consumer(SKConsumer);
 
   if (!swiftTypeContextInfoImpl(*this, UnresolvedInputFile, Offset, Consumer,
-                                Args, fileSystem, error)) {
+                                Args, fileSystem, /*EnableASTCaching=*/false,
+                                error)) {
     SKConsumer.failed(error);
   }
 }

tools/swift-ide-test/swift-ide-test.cpp

@@ -759,11 +759,12 @@ static bool doCodeCompletionImpl(
   CompletionInstance CompletionInst;
   auto isSuccess = CompletionInst.performOperation(
       Invocation, /*Args=*/{}, llvm::vfs::getRealFileSystem(), CleanFile.get(),
-      Offset, Error, CodeCompletionDiagnostics ? &PrintDiags : nullptr,
+      Offset, /*EnableASTCaching=*/false, Error,
-                                                [&](CompilerInstance &CI) {
+      CodeCompletionDiagnostics ? &PrintDiags : nullptr,
-    performCodeCompletionSecondPass(CI.getPersistentParserState(),
+      [&](CompilerInstance &CI) {
-                                    *callbacksFactory);
+        performCodeCompletionSecondPass(CI.getPersistentParserState(),
-  });
+                                        *callbacksFactory);
+      });
   return isSuccess ? 0 : 1;
 }