[PATCH] D61104: [clang][ASTContext] Try to avoid sorting comments for code completion

2019-07-26 Thread Jan Korous via Phabricator via cfe-commits
jkorous abandoned this revision.
jkorous added a comment.

Abandoned in favor of
https://reviews.llvm.org/D65301


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61104/new/

https://reviews.llvm.org/D61104



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D61104: [clang][ASTContext] Try to avoid sorting comments for code completion

2019-05-01 Thread Jan Korous via Phabricator via cfe-commits
jkorous planned changes to this revision.
jkorous added a comment.

@gribozavr thanks for the feedback. I'm rewriting the patch now as I figured 
out my detection of comments preceding declarations is unsound.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61104/new/

https://reviews.llvm.org/D61104



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D61104: [clang][ASTContext] Try to avoid sorting comments for code completion

2019-04-25 Thread Dmitri Gribenko via Phabricator via cfe-commits
gribozavr added inline comments.



Comment at: clang/include/clang/AST/ASTContext.h:800
+  /// The result doesn't contain decls that don't have any comment attached.
+  std::unordered_map getRawCommentsForDeclsNoCache(
+  const std::unordered_map>

Why not DenseMap?



Comment at: clang/include/clang/AST/ASTContext.h:808
+  std::unordered_map
+  getRawCommentsForAnyRedecls(const std::vector ) const;
+

Use ArrayRef.



Comment at: clang/lib/AST/ASTContext.cpp:303
+
+  return Result;
+}

I'm really worried about all the logic duplication here vs. existing code.



Comment at: clang/lib/AST/RawCommentList.cpp:366
+}
+  }
 }

Why is merging in `RawCommentList::addComment` not sufficient?



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3180
   if (IncludeBriefComments) {
+// Try to get the comment if it wasn't provided
+if (!Comment)

There are only a couple of callers of this function, can we change them all to 
provide a comment if it exists?



Comment at: clang/lib/Sema/SemaCodeComplete.cpp:3398
+  for (const auto *const ND : NDs) {
+if (const ObjCMethodDecl *M = dyn_cast(ND)) {
+  if (const ObjCPropertyDecl *PDecl = M->findPropertyDecl()) {

This method decl logic looks out of place here.  It should be pushed down into 
the core logic for attaching comments.


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61104/new/

https://reviews.llvm.org/D61104



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D61104: [clang][ASTContext] Try to avoid sorting comments for code completion

2019-04-24 Thread Jan Korous via Phabricator via cfe-commits
jkorous created this revision.
jkorous added reviewers: gribozavr, arphaman.
Herald added subscribers: cfe-commits, dexonsmith, mgrang.
Herald added a project: clang.

For large number of deserialized comments (~100k) the way how we try to attach 
them to declaration in completion comments is too expensive.

Currently we're sorting all the comments by source location (expensive) and 
later bisecting them for every interesting declaration to find the closest 
comment before and after the declaration documentation comment.

This patch tries to just iterate over unsorted comments and see if there's any 
of the decls we're interested in nearby.


Repository:
  rC Clang

https://reviews.llvm.org/D61104

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/RawCommentList.h
  clang/include/clang/Basic/SourceLocation.h
  clang/include/clang/Sema/CodeCompleteConsumer.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/RawCommentList.cpp
  clang/lib/Sema/SemaCodeComplete.cpp
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/tools/libclang/CIndexCodeCompletion.cpp

Index: clang/tools/libclang/CIndexCodeCompletion.cpp
===
--- clang/tools/libclang/CIndexCodeCompletion.cpp
+++ clang/tools/libclang/CIndexCodeCompletion.cpp
@@ -579,12 +579,46 @@
   StoredResults.reserve(StoredResults.size() + NumResults);
   if (includeFixIts())
 AllocatedResults.FixItsVector.reserve(NumResults);
+
+  auto CanResultHaveComment =
+  [](const CodeCompletionResult ) -> bool {
+const auto  = Result.Kind;
+
+if (Kind == CodeCompletionResult::RK_Declaration ||
+Kind == CodeCompletionResult::RK_Pattern) {
+  if (auto D = Result.getDeclaration()) {
+return true;
+  }
+}
+
+return false;
+  };
+
+  std::vector DeclsThatCanHaveCommentAttached;
   for (unsigned I = 0; I != NumResults; ++I) {
-CodeCompletionString *StoredCompletion
-  = Results[I].CreateCodeCompletionString(S, Context, getAllocator(),
-  getCodeCompletionTUInfo(),
-  includeBriefComments());
-
+if (CanResultHaveComment(Results[I])) {
+  if (auto D = Results[I].getDeclaration()) {
+DeclsThatCanHaveCommentAttached.push_back(D);
+  }
+}
+  }
+
+  auto Cache = getCompletionComments(S.getASTContext(),
+ DeclsThatCanHaveCommentAttached);
+
+  for (unsigned I = 0; I != NumResults; ++I) {
+const RawComment *Comment = nullptr;
+if (CanResultHaveComment(Results[I])) {
+  const auto CachedValueIt = Cache.find(Results[I].getDeclaration());
+  if (CachedValueIt != Cache.end()) {
+Comment = CachedValueIt->second;
+  }
+}
+CodeCompletionString *StoredCompletion =
+Results[I].CreateCodeCompletionString(
+S, Context, getAllocator(), getCodeCompletionTUInfo(),
+includeBriefComments(), Comment);
+
 CXCompletionResult R;
 R.CursorKind = Results[I].CursorKind;
 R.CompletionString = StoredCompletion;
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -3254,7 +3254,11 @@
   auto _ = llvm::make_scope_exit([this] { Stream.ExitBlock(); });
   if (!PP->getPreprocessorOpts().WriteCommentListToPCH)
 return;
-  ArrayRef RawComments = Context->Comments.getComments();
+  ArrayRef RawComments =
+  Context->Comments.getComments(/*Sorted=*/false);
+  Context->Comments.sort();
+  Context->Comments.merge(Context->getLangOpts().CommentOpts);
+
   RecordData Record;
   for (const auto *I : RawComments) {
 Record.clear();
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -9223,7 +9223,6 @@
   NextCursor:
 // De-serialized SourceLocations get negative FileIDs for other modules,
 // potentially invalidating the original order. Sort it again.
-llvm::sort(Comments, BeforeThanCompare(SourceMgr));
 Context.Comments.addDeserializedComments(Comments);
   }
 }
Index: clang/lib/Sema/SemaCodeComplete.cpp
===
--- clang/lib/Sema/SemaCodeComplete.cpp
+++ clang/lib/Sema/SemaCodeComplete.cpp
@@ -3060,9 +3060,9 @@
 CodeCompletionString *CodeCompletionResult::CreateCodeCompletionString(
 Sema , const CodeCompletionContext ,
 CodeCompletionAllocator , CodeCompletionTUInfo ,
-bool IncludeBriefComments) {
+bool IncludeBriefComments, const RawComment *Comment) {
   return