<div dir="ltr">Hi Alex, Sam,<div><br></div><div>I have a few comments regarding the producer part of the API and how it interacts with Context.</div><div><br></div><div>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.</div><div>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.</div><div><br></div><div>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!).</div><div><br></div></div><br><div class="gmail_quote"><div dir="ltr">On Sat, Aug 11, 2018 at 1:23 AM Sam McCall via clangd-dev <<a href="mailto:clangd-dev@lists.llvm.org">clangd-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="auto"><div>Hey Alex,<div dir="auto">In principle no objections, but some design concerns/questions for a larger scope.</div><div dir="auto"><br></div><div dir="auto">- 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.</div><div dir="auto">- 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)</div><div dir="auto">- 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)</div><div dir="auto">- 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.</div><br><br><div class="gmail_quote"><div dir="ltr">On Fri, Aug 10, 2018, 22:20 Alex L via clangd-dev <<a href="mailto:clangd-dev@lists.llvm.org" target="_blank">clangd-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi,<div><br></div><div>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.</div><div><br></div><div>Cheers,</div><div>Alex</div><div><br></div><div><br></div></div>
_______________________________________________<br>
clangd-dev mailing list<br>
<a href="mailto:clangd-dev@lists.llvm.org" rel="noreferrer" target="_blank">clangd-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/clangd-dev" rel="noreferrer noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/clangd-dev</a><br>
</blockquote></div></div></div>
_______________________________________________<br>
clangd-dev mailing list<br>
<a href="mailto:clangd-dev@lists.llvm.org" target="_blank">clangd-dev@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/clangd-dev" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/clangd-dev</a><br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Regards,</div><div>Ilya Biryukov</div></div></div></div></div>