[PATCH] D43377: [clangd] Attach more information about Sema completion to traces

2018-02-19 Thread Phabricator via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE325491: [clangd] Attach more information about Sema 
completion to traces (authored by ibiryukov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D43377?vs=134889=134892#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43377

Files:
  clangd/CodeComplete.cpp


Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -855,9 +855,12 @@
 CompletionList Output;
 semaCodeComplete(std::move(RecorderOwner), Opts.getClangCompleteOpts(),
  SemaCCInput, [&] {
-   if (Recorder.CCSema)
+   if (Recorder.CCSema) {
  Output = runWithSema();
-   else
+ SPAN_ATTACH(
+ Tracer, "sema_completion_kind",
+ 
getCompletionKindString(Recorder.CCContext.getKind()));
+   } else
  log("Code complete: no Sema callback, 0 results");
  });
 


Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -855,9 +855,12 @@
 CompletionList Output;
 semaCodeComplete(std::move(RecorderOwner), Opts.getClangCompleteOpts(),
  SemaCCInput, [&] {
-   if (Recorder.CCSema)
+   if (Recorder.CCSema) {
  Output = runWithSema();
-   else
+ SPAN_ATTACH(
+ Tracer, "sema_completion_kind",
+ getCompletionKindString(Recorder.CCContext.getKind()));
+   } else
  log("Code complete: no Sema callback, 0 results");
  });
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D43377: [clangd] Attach more information about Sema completion to traces

2018-02-19 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 134889.
ilya-biryukov added a comment.

- Rename usage of printCompletionKind to getCompletionKindString
- Remove tracing of the filename, TUScheduler now provides those traces


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43377

Files:
  clangd/CodeComplete.cpp


Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -855,9 +855,12 @@
 CompletionList Output;
 semaCodeComplete(std::move(RecorderOwner), Opts.getClangCompleteOpts(),
  SemaCCInput, [&] {
-   if (Recorder.CCSema)
+   if (Recorder.CCSema) {
  Output = runWithSema();
-   else
+ SPAN_ATTACH(
+ Tracer, "sema_completion_kind",
+ 
getCompletionKindString(Recorder.CCContext.getKind()));
+   } else
  log("Code complete: no Sema callback, 0 results");
  });
 


Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -855,9 +855,12 @@
 CompletionList Output;
 semaCodeComplete(std::move(RecorderOwner), Opts.getClangCompleteOpts(),
  SemaCCInput, [&] {
-   if (Recorder.CCSema)
+   if (Recorder.CCSema) {
  Output = runWithSema();
-   else
+ SPAN_ATTACH(
+ Tracer, "sema_completion_kind",
+ getCompletionKindString(Recorder.CCContext.getKind()));
+   } else
  log("Code complete: no Sema callback, 0 results");
  });
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D43377: [clangd] Attach more information about Sema completion to traces

2018-02-16 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.



Comment at: clangd/CodeComplete.cpp:503
 
+trace::Span Span("Process sema results");
+SPAN_ATTACH(Span, "total_sema_items", NumResults);

sammccall wrote:
> I think this span is going to be tiny (hard to see in some UIs!) and 
> generally not add a lot of value itself.
> The nicest thing would be to just grab the kind out of the CCcontext and 
> attach it to the span in CodeCompleteFlow::run()
> 
> My instinct is that the #items can be dropped entirely - we already log the 
> number of items from Sema considered (before Limit) so all we'd be losing is 
> the number hidden/ineligible/destructor completions we drop here, which seems 
> uninteresting for tracing purposes.
> (But exposing that from the recorder is another option)
You're right, they shouldn't be too important, completion kind is quite useful 
information, though.
I've moved attaching of the completion kind to CodeCompleteFlow::run()



Comment at: clangd/CodeComplete.cpp:504
+trace::Span Span("Process sema results");
+SPAN_ATTACH(Span, "total_sema_items", NumResults);
+SPAN_ATTACH(Span, "sema_completion_kind",

sammccall wrote:
> sema_results_prefilter?
> the filtering is the only difference between this and sema_results logged 
> below
Removed this altogether.



Comment at: clangd/CodeComplete.cpp:918
 
+SPAN_ATTACH(Tracer, "file", SemaCCInput.FileName);
 SPAN_ATTACH(Tracer, "sema_results", NSema);

sammccall wrote:
> FWIW, I have a (delayed, sorry) patch to add a span to all actions run by 
> TUscheduler, which would log the filename. If that lands, *maybe* we don't 
> want it in both places (it'd be the parent span of this one)? But not sure.
LGTM to logging this in TUScheduler. I've added a FIXME to this patch.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43377



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


[PATCH] D43377: [clangd] Attach more information about Sema completion to traces

2018-02-16 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov updated this revision to Diff 134589.
ilya-biryukov marked 2 inline comments as done.
ilya-biryukov added a comment.

- Attach completion kind in CodeCompleteFlow::run().
- Move printCompletionKind closer to CompletionContext::Kind.
- Added FIXME to remove tracing of filename.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43377

Files:
  clangd/CodeComplete.cpp


Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -828,12 +828,17 @@
 CompletionList Output;
 semaCodeComplete(std::move(RecorderOwner), Opts.getClangCompleteOpts(),
  SemaCCInput, [&] {
-   if (Recorder.CCSema)
+   if (Recorder.CCSema) {
  Output = runWithSema();
-   else
+ SPAN_ATTACH(
+ Tracer, "sema_completion_kind",
+ 
printCompletionKind(Recorder.CCContext.getKind()));
+   } else
  log("Code complete: no Sema callback, 0 results");
  });
 
+// FIXME: remove "file" from here after TUScheduler starts logging it.
+SPAN_ATTACH(Tracer, "file", SemaCCInput.FileName);
 SPAN_ATTACH(Tracer, "sema_results", NSema);
 SPAN_ATTACH(Tracer, "index_results", NIndex);
 SPAN_ATTACH(Tracer, "merged_results", NBoth);


Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -828,12 +828,17 @@
 CompletionList Output;
 semaCodeComplete(std::move(RecorderOwner), Opts.getClangCompleteOpts(),
  SemaCCInput, [&] {
-   if (Recorder.CCSema)
+   if (Recorder.CCSema) {
  Output = runWithSema();
-   else
+ SPAN_ATTACH(
+ Tracer, "sema_completion_kind",
+ printCompletionKind(Recorder.CCContext.getKind()));
+   } else
  log("Code complete: no Sema callback, 0 results");
  });
 
+// FIXME: remove "file" from here after TUScheduler starts logging it.
+SPAN_ATTACH(Tracer, "file", SemaCCInput.FileName);
 SPAN_ATTACH(Tracer, "sema_results", NSema);
 SPAN_ATTACH(Tracer, "index_results", NIndex);
 SPAN_ATTACH(Tracer, "merged_results", NBoth);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D43377: [clangd] Attach more information about Sema completion to traces

2018-02-16 Thread Sam McCall via Phabricator via cfe-commits
sammccall added a comment.

This looks really useful! Main suggestion is to drop the added span and attach 
kind to the main span instead. (It's relevant to index too, not just to sema)




Comment at: clangd/CodeComplete.cpp:403
 
+StringRef printCompletionKind(enum CodeCompletionContext::Kind Kind) {
+  using CCKind = enum CodeCompletionContext::Kind;

hmm, this really looks like it belongs next to CodeCompletionContext::Kind?



Comment at: clangd/CodeComplete.cpp:503
 
+trace::Span Span("Process sema results");
+SPAN_ATTACH(Span, "total_sema_items", NumResults);

I think this span is going to be tiny (hard to see in some UIs!) and generally 
not add a lot of value itself.
The nicest thing would be to just grab the kind out of the CCcontext and attach 
it to the span in CodeCompleteFlow::run()

My instinct is that the #items can be dropped entirely - we already log the 
number of items from Sema considered (before Limit) so all we'd be losing is 
the number hidden/ineligible/destructor completions we drop here, which seems 
uninteresting for tracing purposes.
(But exposing that from the recorder is another option)



Comment at: clangd/CodeComplete.cpp:504
+trace::Span Span("Process sema results");
+SPAN_ATTACH(Span, "total_sema_items", NumResults);
+SPAN_ATTACH(Span, "sema_completion_kind",

sema_results_prefilter?
the filtering is the only difference between this and sema_results logged below



Comment at: clangd/CodeComplete.cpp:918
 
+SPAN_ATTACH(Tracer, "file", SemaCCInput.FileName);
 SPAN_ATTACH(Tracer, "sema_results", NSema);

FWIW, I have a (delayed, sorry) patch to add a span to all actions run by 
TUscheduler, which would log the filename. If that lands, *maybe* we don't want 
it in both places (it'd be the parent span of this one)? But not sure.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43377



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


[PATCH] D43377: [clangd] Attach more information about Sema completion to traces

2018-02-16 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov added a comment.

Technically this is NFC, but it has a huge `toString` helper and I'm not sure 
if I chose the appropriate place for logging the filename.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43377



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


[PATCH] D43377: [clangd] Attach more information about Sema completion to traces

2018-02-16 Thread Ilya Biryukov via Phabricator via cfe-commits
ilya-biryukov created this revision.
ilya-biryukov added reviewers: hokein, ioeric, sammccall.
Herald added subscribers: jkorous-apple, klimek.

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D43377

Files:
  clangd/CodeComplete.cpp

Index: clangd/CodeComplete.cpp
===
--- clangd/CodeComplete.cpp
+++ clangd/CodeComplete.cpp
@@ -400,6 +400,82 @@
   return Info.scopesForIndexQuery();
 }
 
+StringRef printCompletionKind(enum CodeCompletionContext::Kind Kind) {
+  using CCKind = enum CodeCompletionContext::Kind;
+  switch (Kind) {
+  case CCKind::CCC_Other:
+return "Other";
+  case CCKind::CCC_OtherWithMacros:
+return "OtherWithMacros";
+  case CCKind::CCC_TopLevel:
+return "TopLevel";
+  case CCKind::CCC_ObjCInterface:
+return "ObjCInterface";
+  case CCKind::CCC_ObjCImplementation:
+return "ObjCImplementation";
+  case CCKind::CCC_ObjCIvarList:
+return "ObjCIvarList";
+  case CCKind::CCC_ClassStructUnion:
+return "ClassStructUnion";
+  case CCKind::CCC_Statement:
+return "Statement";
+  case CCKind::CCC_Expression:
+return "Expression";
+  case CCKind::CCC_ObjCMessageReceiver:
+return "ObjCMessageReceiver";
+  case CCKind::CCC_DotMemberAccess:
+return "DotMemberAccess";
+  case CCKind::CCC_ArrowMemberAccess:
+return "ArrowMemberAccess";
+  case CCKind::CCC_ObjCPropertyAccess:
+return "ObjCPropertyAccess";
+  case CCKind::CCC_EnumTag:
+return "EnumTag";
+  case CCKind::CCC_UnionTag:
+return "UnionTag";
+  case CCKind::CCC_ClassOrStructTag:
+return "ClassOrStructTag";
+  case CCKind::CCC_ObjCProtocolName:
+return "ObjCProtocolName";
+  case CCKind::CCC_Namespace:
+return "Namespace";
+  case CCKind::CCC_Type:
+return "Type";
+  case CCKind::CCC_Name:
+return "Name";
+  case CCKind::CCC_PotentiallyQualifiedName:
+return "PotentiallyQualifiedName";
+  case CCKind::CCC_MacroName:
+return "MacroName";
+  case CCKind::CCC_MacroNameUse:
+return "MacroNameUse";
+  case CCKind::CCC_PreprocessorExpression:
+return "PreprocessorExpression";
+  case CCKind::CCC_PreprocessorDirective:
+return "PreprocessorDirective";
+  case CCKind::CCC_NaturalLanguage:
+return "NaturalLanguage";
+  case CCKind::CCC_SelectorName:
+return "SelectorName";
+  case CCKind::CCC_TypeQualifiers:
+return "TypeQualifiers";
+  case CCKind::CCC_ParenthesizedExpression:
+return "ParenthesizedExpression";
+  case CCKind::CCC_ObjCInstanceMessage:
+return "ObjCInstanceMessage";
+  case CCKind::CCC_ObjCClassMessage:
+return "ObjCClassMessage";
+  case CCKind::CCC_ObjCInterfaceName:
+return "ObjCInterfaceName";
+  case CCKind::CCC_ObjCCategoryName:
+return "ObjCCategoryName";
+  case CCKind::CCC_Recovery:
+return "Recovery";
+  }
+  // used only for tracing, so we don't want to crash or have UB here.
+  return "unknown completion kind";
+}
+
 // The CompletionRecorder captures Sema code-complete output, including context.
 // It filters out ignored results (but doesn't apply fuzzy-filtering yet).
 // It doesn't do scoring or conversion to CompletionItem yet, as we want to
@@ -424,6 +500,11 @@
 CCSema = 
 CCContext = Context;
 
+trace::Span Span("Process sema results");
+SPAN_ATTACH(Span, "total_sema_items", NumResults);
+SPAN_ATTACH(Span, "sema_completion_kind",
+printCompletionKind(Context.getKind()));
+
 // Retain the results we might want.
 for (unsigned I = 0; I < NumResults; ++I) {
   auto  = InResults[I];
@@ -834,6 +915,7 @@
  log("Code complete: no Sema callback, 0 results");
  });
 
+SPAN_ATTACH(Tracer, "file", SemaCCInput.FileName);
 SPAN_ATTACH(Tracer, "sema_results", NSema);
 SPAN_ATTACH(Tracer, "index_results", NIndex);
 SPAN_ATTACH(Tracer, "merged_results", NBoth);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits