[clang-tools-extra] 8f1de22 - [clangd] Stop capturing trace args if the tracer doesn't need them.
Sam McCall via cfe-commits
cfe-commits at lists.llvm.org
Mon Oct 12 04:44:34 PDT 2020
Author: Sam McCall
Date: 2020-10-12T13:43:05+02:00
New Revision: 8f1de22c7681a21fcdbe2d8c39de7698baab724a
URL: https://github.com/llvm/llvm-project/commit/8f1de22c7681a21fcdbe2d8c39de7698baab724a
DIFF: https://github.com/llvm/llvm-project/commit/8f1de22c7681a21fcdbe2d8c39de7698baab724a.diff
LOG: [clangd] Stop capturing trace args if the tracer doesn't need them.
The tracer is now expected to allocate+free the args itself.
Differential Revision: https://reviews.llvm.org/D89135
Added:
Modified:
clang-tools-extra/clangd/support/Trace.cpp
clang-tools-extra/clangd/support/Trace.h
clang-tools-extra/clangd/unittests/support/TraceTests.cpp
Removed:
################################################################################
diff --git a/clang-tools-extra/clangd/support/Trace.cpp b/clang-tools-extra/clangd/support/Trace.cpp
index 7ab09cd23e6a..89d65e686ebc 100644
--- a/clang-tools-extra/clangd/support/Trace.cpp
+++ b/clang-tools-extra/clangd/support/Trace.cpp
@@ -53,9 +53,12 @@ class JSONTracer : public EventTracer {
// 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<JSONSpan>(this, Name, Args));
+ Context beginSpan(
+ llvm::StringRef Name,
+ llvm::function_ref<void(llvm::json::Object *)> AttachDetails) override {
+ auto JS = std::make_unique<JSONSpan>(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 @@ class JSONTracer : public EventTracer {
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
diff erent thread, so we need to capture now.
Tracer->captureThreadMetadata();
@@ -125,7 +128,7 @@ class JSONTracer : public EventTracer {
// 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 @@ class JSONTracer : public EventTracer {
// May be called by any thread.
void markEnded() { EndTime = Tracer->timestamp(); }
+ llvm::json::Object Args;
+
private:
static int64_t nextID() {
static std::atomic<int64_t> Next = {0};
@@ -144,7 +149,6 @@ class JSONTracer : public EventTracer {
std::string Name;
uint64_t TID;
JSONTracer *Tracer;
- llvm::json::Object *Args;
};
static Key<std::unique_ptr<JSONSpan>> SpanKey;
@@ -277,12 +281,11 @@ void log(const llvm::Twine &Message) {
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<Context, llvm::json::Object *>
+makeSpanContext(llvm::Twine Name, const Metric &LatencyMetric) {
if (!T)
- return Context::current().clone();
- WithContextValue WithArgs{std::unique_ptr<llvm::json::Object>(Args)};
+ return std::make_pair(Context::current().clone(), nullptr);
llvm::Optional<WithContextValue> WithLatency;
using Clock = std::chrono::high_resolution_clock;
WithLatency.emplace(llvm::make_scope_exit(
@@ -293,9 +296,15 @@ static Context makeSpanContext(llvm::Twine Name, llvm::json::Object *Args,
.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 @@ constexpr Metric SpanLatency("span_latency", Metric::Distribution, "span_name");
// 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<Context, llvm::json::Object *> Pair)
+ : Args(Pair.second), RestoreCtx(std::move(Pair.first)) {}
Span::~Span() {
if (T)
@@ -323,7 +333,9 @@ void Metric::record(double Value, llvm::StringRef Label) const {
T->record(*this, Value, Label);
}
-Context EventTracer::beginSpan(llvm::StringRef Name, llvm::json::Object *Args) {
+Context EventTracer::beginSpan(
+ llvm::StringRef Name,
+ llvm::function_ref<void(llvm::json::Object *)> AttachDetails) {
return Context::current().clone();
}
} // namespace trace
diff --git a/clang-tools-extra/clangd/support/Trace.h b/clang-tools-extra/clangd/support/Trace.h
index 9dc397a84b74..7bcfbef7aad2 100644
--- a/clang-tools-extra/clangd/support/Trace.h
+++ b/clang-tools-extra/clangd/support/Trace.h
@@ -79,8 +79,13 @@ class EventTracer {
/// 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<void(llvm::json::Object *)> 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 @@ class Span {
llvm::json::Object *const Args;
private:
+ // Awkward constructor works around constant initialization.
+ Span(std::pair<Context, llvm::json::Object *>);
WithContext RestoreCtx;
};
diff --git a/clang-tools-extra/clangd/unittests/support/TraceTests.cpp b/clang-tools-extra/clangd/unittests/support/TraceTests.cpp
index 116d9fcee02c..bb515edd7253 100644
--- a/clang-tools-extra/clangd/unittests/support/TraceTests.cpp
+++ b/clang-tools-extra/clangd/unittests/support/TraceTests.cpp
@@ -186,6 +186,11 @@ TEST_F(CSVMetricsTracerTest, Escaping) {
StartsWith("d,dist,\"a\nb\",1"), ""));
}
+TEST_F(CSVMetricsTracerTest, IgnoresArgs) {
+ trace::Span Tracer("Foo");
+ EXPECT_EQ(nullptr, Tracer.Args);
+}
+
} // namespace
} // namespace clangd
} // namespace clang
More information about the cfe-commits
mailing list