<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Mon, 13 Aug 2018 at 08:05, Ilya Biryukov <<a href="mailto:ibiryukov@google.com">ibiryukov@google.com</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 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></blockquote><div><br></div><div>Thanks! I think that makes sense. I will work on separating the Context from the tracing itself first.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><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" 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="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></div></div></blockquote></div></blockquote><div><br></div><div>Thanks! I'll take your points into consideration. I think Ilya is right about leaving the Context private to Clangd as well.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="auto"><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="m_3147076856090767317gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Regards,</div><div>Ilya Biryukov</div></div></div></div></div>
</blockquote></div></div>