diff --git a/clang-tools-extra/clangd/support/Trace.h b/clang-tools-extra/clangd/support/Trace.h --- a/clang-tools-extra/clangd/support/Trace.h +++ b/clang-tools-extra/clangd/support/Trace.h @@ -79,8 +79,13 @@ /// 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(llvm::StringRef Name, llvm::json::Object *Args); + /// The tracer may capture event details provided in SPAN_ATTACH() calls. + /// In this case it should call AttachDetails(), and pass in an empty Object + /// to hold them. This Object should be owned by the context, and the data + /// will be complete by the time the context is destroyed. + virtual Context + beginSpan(llvm::StringRef Name, + llvm::function_ref AttachDetails); // Called when a Span is destroyed (it may still be active on other threads). // beginSpan() and endSpan() will always form a proper stack on each thread. // The Context returned by beginSpan is active, but Args is not ready. @@ -146,6 +151,8 @@ llvm::json::Object *const Args; private: + // Awkward constructor works around constant initialization. + Span(std::pair); WithContext RestoreCtx; }; diff --git a/clang-tools-extra/clangd/support/Trace.cpp b/clang-tools-extra/clangd/support/Trace.cpp --- a/clang-tools-extra/clangd/support/Trace.cpp +++ b/clang-tools-extra/clangd/support/Trace.cpp @@ -53,9 +53,12 @@ // We stash a Span object in the context. It will record the start/end, // and this also allows us to look up the parent Span's information. - Context beginSpan(llvm::StringRef Name, llvm::json::Object *Args) override { - return Context::current().derive( - SpanKey, std::make_unique(this, Name, Args)); + Context beginSpan( + llvm::StringRef Name, + llvm::function_ref AttachDetails) override { + auto JS = std::make_unique(this, Name); + AttachDetails(&JS->Args); + return Context::current().derive(SpanKey, std::move(JS)); } // Trace viewer requires each thread to properly stack events. @@ -85,9 +88,9 @@ private: class JSONSpan { public: - JSONSpan(JSONTracer *Tracer, llvm::StringRef Name, llvm::json::Object *Args) + JSONSpan(JSONTracer *Tracer, llvm::StringRef Name) : StartTime(Tracer->timestamp()), EndTime(0), Name(Name), - TID(llvm::get_threadid()), Tracer(Tracer), Args(Args) { + TID(llvm::get_threadid()), Tracer(Tracer) { // ~JSONSpan() may run in a different thread, so we need to capture now. Tracer->captureThreadMetadata(); @@ -125,7 +128,7 @@ // Finally, record the event (ending at EndTime, not timestamp())! Tracer->jsonEvent("X", llvm::json::Object{{"name", std::move(Name)}, - {"args", std::move(*Args)}, + {"args", std::move(Args)}, {"dur", EndTime - StartTime}}, TID, StartTime); } @@ -133,6 +136,8 @@ // May be called by any thread. void markEnded() { EndTime = Tracer->timestamp(); } + llvm::json::Object Args; + private: static int64_t nextID() { static std::atomic Next = {0}; @@ -144,7 +149,6 @@ std::string Name; uint64_t TID; JSONTracer *Tracer; - llvm::json::Object *Args; }; static Key> SpanKey; @@ -277,12 +281,11 @@ T->instant("Log", llvm::json::Object{{"Message", Message.str()}}); } -// Returned context owns Args. -static Context makeSpanContext(llvm::Twine Name, llvm::json::Object *Args, - const Metric &LatencyMetric) { +// The JSON object is event args (owned by context), if the tracer wants them. +static std::pair +makeSpanContext(llvm::Twine Name, const Metric &LatencyMetric) { if (!T) - return Context::current().clone(); - WithContextValue WithArgs{std::unique_ptr(Args)}; + return std::make_pair(Context::current().clone(), nullptr); llvm::Optional WithLatency; using Clock = std::chrono::high_resolution_clock; WithLatency.emplace(llvm::make_scope_exit( @@ -293,9 +296,15 @@ .count(), Name); })); - return T->beginSpan(Name.isSingleStringRef() ? Name.getSingleStringRef() - : llvm::StringRef(Name.str()), - Args); + llvm::json::Object *Args = nullptr; + Context Ctx = T->beginSpan( + Name.isSingleStringRef() ? Name.getSingleStringRef() + : llvm::StringRef(Name.str()), + [&](llvm::json::Object *A) { + assert(A && A->empty() && "Invalid AttachDetails() placeholder!"); + Args = A; + }); + return std::make_pair(std::move(Ctx), Args); } // Fallback metric that measures latencies for spans without an explicit latency @@ -307,8 +316,9 @@ // beginSpan() context is destroyed, when the tracing engine will consume them. Span::Span(llvm::Twine Name) : Span(Name, SpanLatency) {} Span::Span(llvm::Twine Name, const Metric &LatencyMetric) - : Args(T ? new llvm::json::Object() : nullptr), - RestoreCtx(makeSpanContext(Name, Args, LatencyMetric)) {} + : Span(makeSpanContext(Name, LatencyMetric)) {} +Span::Span(std::pair Pair) + : Args(Pair.second), RestoreCtx(std::move(Pair.first)) {} Span::~Span() { if (T) @@ -323,7 +333,9 @@ T->record(*this, Value, Label); } -Context EventTracer::beginSpan(llvm::StringRef Name, llvm::json::Object *Args) { +Context EventTracer::beginSpan( + llvm::StringRef Name, + llvm::function_ref AttachDetails) { return Context::current().clone(); } } // namespace trace diff --git a/clang-tools-extra/clangd/unittests/support/TraceTests.cpp b/clang-tools-extra/clangd/unittests/support/TraceTests.cpp --- a/clang-tools-extra/clangd/unittests/support/TraceTests.cpp +++ b/clang-tools-extra/clangd/unittests/support/TraceTests.cpp @@ -186,6 +186,11 @@ StartsWith("d,dist,\"a\nb\",1"), "")); } +TEST_F(CSVMetricsTracerTest, IgnoresArgs) { + trace::Span Tracer("Foo"); + EXPECT_EQ(nullptr, Tracer.Args); +} + } // namespace } // namespace clangd } // namespace clang