Moving clangd/Trace over to LLVM

Hi,

I would like to move the Tracing support from Clangd over to LLVM, as I would like to add some tracing support to Clang itself as well. Are there any objections to this from the Clangd side? I will make a follow-up post on llvm-dev with a patch if there are no objections.

Cheers,
Alex

Hey Alex,
In principle no objections, but some design concerns/questions for a larger scope.

  • the global registration of the EventTracer is awkward and may not be the right design for all environments who might otherwise want to consume events. Didn’t spend a lot of time digging for alternatives here since it was sufficient for clangd.
  • tracer depends on Context to handle cross-thread interactions, which is another possibly-controversial piece. (I’m more confident the design of Context is right though)
  • contexts need to be propagated across thread boundaries when work is transferred. Clangd’s TUScheduler does this, but the wider LLVM universe does not. (More generally: these APIs may not be very useful if they’re not used consistently)
  • both tracer and context propagation were designed as extension points for weird clangd embedders, and we have nontrivial uses of these (happy to give details). So if these APIs change a lot to make them more suitable for LLVM, we need to make sure they still work for those original purposes.

Hi Alex, Sam,

I have a few comments regarding the producer part of the API and how it interacts with Context.

If I were to move the Tracing API out of clangd, I would probably decouple it from the Context first and leave the Context private to clangd.
There are ways to do that, e.g. by 1) introducing explicit API to allow keeping the traces alive after the corresponding RAII “Span” dies, 2) using this API in clangd to propagate traces across thread boundaries using clangd’s Context.

Context has all the same problems that global variables have and I personally find it’s too easy to misuse (specifically since it propagates across threads!).

Hi Alex, Sam,

I have a few comments regarding the producer part of the API and how it interacts with Context.

If I were to move the Tracing API out of clangd, I would probably decouple it from the Context first and leave the Context private to clangd.
There are ways to do that, e.g. by 1) introducing explicit API to allow keeping the traces alive after the corresponding RAII “Span” dies, 2) using this API in clangd to propagate traces across thread boundaries using clangd’s Context.

Thanks! I think that makes sense. I will work on separating the Context from the tracing itself first.

Context has all the same problems that global variables have and I personally find it’s too easy to misuse (specifically since it propagates across threads!).

Hey Alex,
In principle no objections, but some design concerns/questions for a larger scope.

  • the global registration of the EventTracer is awkward and may not be the right design for all environments who might otherwise want to consume events. Didn’t spend a lot of time digging for alternatives here since it was sufficient for clangd.
  • tracer depends on Context to handle cross-thread interactions, which is another possibly-controversial piece. (I’m more confident the design of Context is right though)
  • contexts need to be propagated across thread boundaries when work is transferred. Clangd’s TUScheduler does this, but the wider LLVM universe does not. (More generally: these APIs may not be very useful if they’re not used consistently)
  • both tracer and context propagation were designed as extension points for weird clangd embedders, and we have nontrivial uses of these (happy to give details). So if these APIs change a lot to make them more suitable for LLVM, we need to make sure they still work for those original purposes.

Thanks! I’ll take your points into consideration. I think Ilya is right about leaving the Context private to Clangd as well.