[PATCH] D43377: [clangd] Attach more information about Sema completion to traces
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
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
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
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
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
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
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