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
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
