Index: clangd/ClangdUnit.cpp =================================================================== --- clangd/ClangdUnit.cpp +++ clangd/ClangdUnit.cpp @@ -550,15 +550,16 @@ CI->getFrontendOpts().SkipFunctionBodies = false; if (BuiltPreamble) { - log(Ctx, "Built preamble of size " + Twine(BuiltPreamble->getSize()) + - " for file " + Twine(That->FileName)); + log(Tracer.Ctx, "Built preamble of size " + + Twine(BuiltPreamble->getSize()) + " for file " + + Twine(That->FileName)); return std::make_shared( std::move(*BuiltPreamble), SerializedDeclsCollector.takeTopLevelDeclIDs(), std::move(PreambleDiags)); } else { - log(Ctx, + log(Tracer.Ctx, "Could not build a preamble for file " + Twine(That->FileName)); return nullptr; } @@ -589,8 +590,9 @@ { trace::Span Tracer(Ctx, "Build"); SPAN_ATTACH(Tracer, "File", That->FileName); - NewAST = ParsedAST::Build(Ctx, std::move(CI), std::move(NewPreamble), - std::move(ContentsBuffer), PCHs, Inputs.FS); + NewAST = + ParsedAST::Build(Tracer.Ctx, std::move(CI), std::move(NewPreamble), + std::move(ContentsBuffer), PCHs, Inputs.FS); } if (NewAST) { Index: clangd/Context.h =================================================================== --- clangd/Context.h +++ clangd/Context.h @@ -84,6 +84,9 @@ /// /// Keys are typically used across multiple functions, so most of the time you /// would want to make them static class members or global variables. +/// +/// FIXME: Rather than manual plumbing, pass Context using thread-local storage +/// by default, and make thread boundaries deal with propagation explicitly. class Context { public: /// Returns an empty context that contains no data. Useful for calling @@ -150,6 +153,18 @@ std::move(Value))})); } + /// Derives a child context, using an anonymous key. + /// Intended for objects stored only for their destructor's side-effect. + template Context derive(Type &&Value) const & { + static Key::type> Private; + return derive(Private, std::forward(Value)); + } + + template Context derive(Type &&Value) && { + static Key::type> Private; + return std::move(this)->derive(Private, std::forward(Value)); + } + /// Clone this context object. Context clone() const; Index: clangd/Function.h =================================================================== --- clangd/Function.h +++ clangd/Function.h @@ -144,7 +144,7 @@ static_assert(std::is_same::type, Func>::value, "Func must be decayed"); - ScopeExitGuard(Func F) : F(std::move(F)) {} + ScopeExitGuard(Func F) : F(llvm::make_unique(std::move(F))) {} ~ScopeExitGuard() { if (!F) return; @@ -159,7 +159,7 @@ ScopeExitGuard &operator=(ScopeExitGuard &&Other) = default; private: - llvm::Optional F; + std::unique_ptr F; }; } // namespace detail Index: clangd/JSONRPCDispatcher.cpp =================================================================== --- clangd/JSONRPCDispatcher.cpp +++ clangd/JSONRPCDispatcher.cpp @@ -20,9 +20,28 @@ using namespace clangd; namespace { -static Key> RequestSpan; static Key RequestID; static Key RequestOut; + +// We stash the Span.Args for the enclosing request in the context, +// so we can attach other information as it's available (e.g. response). +// Because Span isn't threadsafe, we also need to lock it. +struct RequestArgs { + RequestArgs(json::obj *Args) : Args(Args) {} + std::mutex Mu; + json::obj *Args; +}; +static Key> RequestArgsKey; +template +// If Ctx is within a request, and the tracer is interested in args, call +// F with the json::obj which we can add data to, and the lock held. +void FillRequestInfo(const Context &Ctx, Func &&F) { + auto *RequestArgs = Ctx.get(RequestArgsKey); + if (!RequestArgs || !*RequestArgs || !(*RequestArgs)->Args) + return; + std::lock_guard Lock((*RequestArgs)->Mu); + F(*(*RequestArgs)->Args); +} } // namespace void JSONOutput::writeMessage(const json::Expr &Message) { @@ -66,8 +85,9 @@ return; } - if (auto *Span = Ctx.get(RequestSpan)) - SPAN_ATTACH(**Span, "Reply", Result); + FillRequestInfo(Ctx, [&](json::obj &Args){ + Args["Reply"] = Result; + }); Ctx.getExisting(RequestOut) ->writeMessage(json::obj{ @@ -80,10 +100,10 @@ void clangd::replyError(const Context &Ctx, ErrorCode code, const llvm::StringRef &Message) { log(Ctx, "Error " + Twine(static_cast(code)) + ": " + Message); - if (auto *Span = Ctx.get(RequestSpan)) - SPAN_ATTACH(**Span, "Error", - (json::obj{{"code", static_cast(code)}, - {"message", Message.str()}})); + FillRequestInfo(Ctx, [&](json::obj &Args) { + Args["Error"] = + json::obj{{"code", static_cast(code)}, {"message", Message.str()}}; + }); if (auto ID = Ctx.get(RequestID)) { Ctx.getExisting(RequestOut) @@ -99,9 +119,9 @@ void clangd::call(const Context &Ctx, StringRef Method, json::Expr &&Params) { // FIXME: Generate/Increment IDs for every request so that we can get proper // replies once we need to. - if (auto *Span = Ctx.get(RequestSpan)) - SPAN_ATTACH(**Span, "Call", - (json::obj{{"method", Method.str()}, {"params", Params}})); + FillRequestInfo(Ctx, [&](json::obj &Args) { + Args["Call"] = json::obj{{"method", Method.str()}, {"params", Params}}; + }); Ctx.getExisting(RequestOut) ->writeMessage(json::obj{ {"jsonrpc", "2.0"}, @@ -143,15 +163,15 @@ Ctx = std::move(Ctx).derive(RequestID, *ID); // Create a tracing Span covering the whole request lifetime. - auto Tracer = llvm::make_unique(Ctx, *Method); + trace::Span Tracer(Ctx, *Method); if (ID) - SPAN_ATTACH(*Tracer, "ID", *ID); - SPAN_ATTACH(*Tracer, "Params", Params); - - // Update Ctx to include Tracer. - Ctx = std::move(Ctx).derive(RequestSpan, std::move(Tracer)); + SPAN_ATTACH(Tracer, "ID", *ID); + SPAN_ATTACH(Tracer, "Params", Params); - Handler(std::move(Ctx), std::move(Params)); + // Stash a reference to the span args, so later calls can add metadata. + Handler(Tracer.Ctx.derive(RequestArgsKey, + llvm::make_unique(Tracer.Args)), + std::move(Params)); return true; } Index: clangd/Trace.h =================================================================== --- clangd/Trace.h +++ clangd/Trace.h @@ -32,17 +32,15 @@ /// Implmentations of this interface must be thread-safe. class EventTracer { public: - /// A callback executed when an event with duration ends. Args represent data - /// that was attached to the event via SPAN_ATTACH. - using EndEventCallback = UniqueFunction; - virtual ~EventTracer() = default; - /// Called when event that has a duration starts. The returned callback will - /// be executed when the event ends. \p Name is a descriptive name - /// of the event that was passed to Span constructor. - virtual EndEventCallback beginSpan(const Context &Ctx, - llvm::StringRef Name) = 0; + /// Called when event that has a duration starts. \p Name describes the event. + /// Returns a derived context that will be destroyed when the event ends. + /// Usually implementations will store an object in the returned context + /// whose destructor records the end of the event. + /// The args are *Args, only complete when the event ends. + virtual Context beginSpan(const Context &Ctx, llvm::StringRef Name, + json::obj *Args) = 0; /// Called for instant events. virtual void instant(const Context &Ctx, llvm::StringRef Name, @@ -70,30 +68,35 @@ void log(const Context &Ctx, const llvm::Twine &Name); /// Records an event whose duration is the lifetime of the Span object. +/// This lifetime is extended when the span's context is reused. +/// /// This is the main public interface for producing tracing events. /// /// Arbitrary JSON metadata can be attached while this span is active: /// SPAN_ATTACH(MySpan, "Payload", SomeJSONExpr); +/// /// SomeJSONExpr is evaluated and copied only if actually needed. class Span { -public: + public: Span(const Context &Ctx, llvm::StringRef Name); - ~Span(); - /// Returns mutable span metadata if this span is interested. + /// Mutable metadata, if this span is interested. /// Prefer to use SPAN_ATTACH rather than accessing this directly. - json::obj *args() { return Args.get(); } - -private: - std::unique_ptr Args; - EventTracer::EndEventCallback Callback; + json::obj * const Args; + /// Propagating this context will keep the span alive. + const Context Ctx; }; +/// Returns mutable span metadata if this span is interested. +/// Prefer to use SPAN_ATTACH rather than accessing this directly. +json::obj *spanArgs(const Context &Ctx); + +/// Attach a key-value pair to a Span event. +/// This is not threadsafe when used with the same Span. #define SPAN_ATTACH(S, Name, Expr) \ do { \ - if ((S).args() != nullptr) { \ - (*((S).args()))[(Name)] = (Expr); \ - } \ + if (auto *Args = (S).Args) \ + (*Args)[Name] = Expr; \ } while (0) } // namespace trace Index: clangd/Trace.cpp =================================================================== --- clangd/Trace.cpp +++ clangd/Trace.cpp @@ -7,6 +7,8 @@ // //===----------------------------------------------------------------------===// +#include "Context.h" +#include "Function.h" #include "Trace.h" #include "llvm/ADT/DenseSet.h" #include "llvm/Support/Chrono.h" @@ -43,14 +45,12 @@ Out.flush(); } - EndEventCallback beginSpan(const Context &Ctx, - llvm::StringRef Name) override { + Context beginSpan(const Context &Ctx, llvm::StringRef Name, + json::obj *Args) override { jsonEvent("B", json::obj{{"name", Name}}); - - // The callback that will run when event ends. - return [this](json::Expr &&Args) { - jsonEvent("E", json::obj{{"args", std::move(Args)}}); - }; + return Ctx.derive(onScopeExit([this, Args] { + jsonEvent("E", json::obj{{"args", std::move(*Args)}}); + })); } void instant(const Context &Ctx, llvm::StringRef Name, @@ -125,24 +125,17 @@ T->instant(Ctx, "Log", json::obj{{"Message", Message.str()}}); } -Span::Span(const Context &Ctx, llvm::StringRef Name) { - if (!T) - return; - - Callback = T->beginSpan(Ctx, Name); - if (!Callback) - return; - - Args = llvm::make_unique(); -} - -Span::~Span() { - if (!Callback) - return; - - assert(Args && "Args must be non-null if Callback is defined"); - Callback(std::move(*Args)); -} +static Key> kSpanArgs; + +// Span keeps a non-owning pointer to the args, which is how users access them. +// The args are owned by the context though. They stick around until the +// beginSpan() context is destroyed, when the tracing engine will consume them. +Span::Span(const Context &Ctx, llvm::StringRef Name) + : Args(T ? new json::obj() : nullptr), + Ctx(T ? T->beginSpan( + Ctx.derive(kSpanArgs, std::unique_ptr(Args)), Name, + Args) + : Ctx.derive(kSpanArgs, nullptr)) {} } // namespace trace } // namespace clangd Index: unittests/clangd/TraceTests.cpp =================================================================== --- unittests/clangd/TraceTests.cpp +++ unittests/clangd/TraceTests.cpp @@ -78,8 +78,8 @@ auto JSONTracer = trace::createJSONTracer(OS); trace::Session Session(*JSONTracer); { - trace::Span S(Context::empty(), "A"); - trace::log(Context::empty(), "B"); + trace::Span Tracer(Context::empty(), "A"); + trace::log(Tracer.Ctx, "B"); } }