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

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Feb 16 03:28:12 PST 2018


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





More information about the cfe-commits mailing list