[PATCH] D40489: [clangd] Changed tracing interfaces

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rCTE320708: [clangd] Changed tracing interfaces (authored by 
ibiryukov, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D40489?vs=126957=126959#toc

Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40489

Files:
  clangd/Trace.cpp
  clangd/Trace.h

Index: clangd/Trace.cpp
===
--- clangd/Trace.cpp
+++ clangd/Trace.cpp
@@ -44,10 +44,24 @@
 Out.flush();
   }
 
+  EndEventCallback beginSpan(const Context ,
+ llvm::StringRef Name) override {
+jsonEvent("B", json::obj{{"name", Name}});
+
+// The callback that will run when event ends.
+return [this](json::Expr &) {
+  jsonEvent("E", json::obj{{"args", std::move(Args)}});
+};
+  }
+
+  void instant(const Context , llvm::StringRef Name,
+   json::obj &) override {
+jsonEvent("i", json::obj{{"name", Name}, {"args", std::move(Args)}});
+  }
+
   // Record an event on the current thread. ph, pid, tid, ts are set.
   // Contents must be a list of the other JSON key/values.
-  void event(const Context , StringRef Phase,
- json::obj &) override {
+  void jsonEvent(StringRef Phase, json::obj &) {
 uint64_t TID = get_threadid();
 std::lock_guard Lock(Mu);
 // If we haven't already, emit metadata describing this thread.
@@ -109,30 +123,26 @@
 void log(const Context , const Twine ) {
   if (!T)
 return;
-  T->event(Ctx, "i",
-   json::obj{
-   {"name", "Log"},
-   {"args", json::obj{{"Message", Message.str()}}},
-   });
+  T->instant(Ctx, "Log", json::obj{{"Message", Message.str()}});
 }
 
-Span::Span(const Context , std::string Name) {
+Span::Span(const Context , llvm::StringRef Name) {
   if (!T)
 return;
-  // Clone the context, so that the original Context can be moved.
-  this->Ctx.emplace(Ctx.clone());
 
-  T->event(*this->Ctx, "B", json::obj{{"name", std::move(Name)}});
+  Callback = T->beginSpan(Ctx, Name);
+  if (!Callback)
+return;
+
   Args = llvm::make_unique();
 }
 
 Span::~Span() {
-  if (!T)
+  if (!Callback)
 return;
-  if (!Args)
-Args = llvm::make_unique();
-  T->event(*Ctx, "E",
-   Args ? json::obj{{"args", std::move(*Args)}} : json::obj{});
+
+  assert(Args && "Args must be non-null if Callback is defined");
+  Callback(std::move(*Args));
 }
 
 } // namespace trace
Index: clangd/Trace.h
===
--- clangd/Trace.h
+++ clangd/Trace.h
@@ -19,6 +19,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRACE_H_
 
 #include "Context.h"
+#include "Function.h"
 #include "JSONExpr.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/raw_ostream.h"
@@ -28,12 +29,24 @@
 namespace trace {
 
 /// A consumer of trace events. The events are produced by Spans and trace::log.
+/// 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;
-  /// Consume a trace event.
-  virtual void event(const Context , llvm::StringRef Phase,
- json::obj &) = 0;
+
+  /// 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 ,
+ llvm::StringRef Name) = 0;
+
+  /// Called for instant events.
+  virtual void instant(const Context , llvm::StringRef Name,
+   json::obj &) = 0;
 };
 
 /// Sets up a global EventTracer that consumes events produced by Span and
@@ -50,9 +63,6 @@
 ///
 /// The format is documented here:
 /// https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview
-///
-/// The implementation supports concurrent calls and can be used as a global
-/// tracer (i.e., can be put into a global Context).
 std::unique_ptr createJSONTracer(llvm::raw_ostream ,
   bool Pretty = false);
 
@@ -67,16 +77,16 @@
 /// SomeJSONExpr is evaluated and copied only if actually needed.
 class Span {
 public:
-  Span(const Context , std::string Name);
+  Span(const Context , llvm::StringRef Name);
   ~Span();
 
   /// Returns mutable span metadata if this span is interested.
   /// Prefer to use SPAN_ATTACH rather than accessing this directly.
   json::obj *args() { return Args.get(); }
 
 private:
-  llvm::Optional Ctx;
   std::unique_ptr Args;
+  EventTracer::EndEventCallback Callback;
 };
 
 #define SPAN_ATTACH(S, Name, Expr) \

[PATCH] D40489: [clangd] Changed tracing interfaces

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 126957.
ilya-biryukov added a comment.

Merged with head


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40489

Files:
  clangd/Trace.cpp
  clangd/Trace.h

Index: clangd/Trace.h
===
--- clangd/Trace.h
+++ clangd/Trace.h
@@ -19,6 +19,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRACE_H_
 
 #include "Context.h"
+#include "Function.h"
 #include "JSONExpr.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/raw_ostream.h"
@@ -28,12 +29,24 @@
 namespace trace {
 
 /// A consumer of trace events. The events are produced by Spans and trace::log.
+/// 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;
-  /// Consume a trace event.
-  virtual void event(const Context , llvm::StringRef Phase,
- json::obj &) = 0;
+
+  /// 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 ,
+ llvm::StringRef Name) = 0;
+
+  /// Called for instant events.
+  virtual void instant(const Context , llvm::StringRef Name,
+   json::obj &) = 0;
 };
 
 /// Sets up a global EventTracer that consumes events produced by Span and
@@ -50,9 +63,6 @@
 ///
 /// The format is documented here:
 /// https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview
-///
-/// The implementation supports concurrent calls and can be used as a global
-/// tracer (i.e., can be put into a global Context).
 std::unique_ptr createJSONTracer(llvm::raw_ostream ,
   bool Pretty = false);
 
@@ -67,16 +77,16 @@
 /// SomeJSONExpr is evaluated and copied only if actually needed.
 class Span {
 public:
-  Span(const Context , std::string Name);
+  Span(const Context , llvm::StringRef Name);
   ~Span();
 
   /// Returns mutable span metadata if this span is interested.
   /// Prefer to use SPAN_ATTACH rather than accessing this directly.
   json::obj *args() { return Args.get(); }
 
 private:
-  llvm::Optional Ctx;
   std::unique_ptr Args;
+  EventTracer::EndEventCallback Callback;
 };
 
 #define SPAN_ATTACH(S, Name, Expr) \
Index: clangd/Trace.cpp
===
--- clangd/Trace.cpp
+++ clangd/Trace.cpp
@@ -44,10 +44,24 @@
 Out.flush();
   }
 
+  EndEventCallback beginSpan(const Context ,
+ llvm::StringRef Name) override {
+jsonEvent("B", json::obj{{"name", Name}});
+
+// The callback that will run when event ends.
+return [this](json::Expr &) {
+  jsonEvent("E", json::obj{{"args", std::move(Args)}});
+};
+  }
+
+  void instant(const Context , llvm::StringRef Name,
+   json::obj &) override {
+jsonEvent("i", json::obj{{"name", Name}, {"args", std::move(Args)}});
+  }
+
   // Record an event on the current thread. ph, pid, tid, ts are set.
   // Contents must be a list of the other JSON key/values.
-  void event(const Context , StringRef Phase,
- json::obj &) override {
+  void jsonEvent(StringRef Phase, json::obj &) {
 uint64_t TID = get_threadid();
 std::lock_guard Lock(Mu);
 // If we haven't already, emit metadata describing this thread.
@@ -109,30 +123,26 @@
 void log(const Context , const Twine ) {
   if (!T)
 return;
-  T->event(Ctx, "i",
-   json::obj{
-   {"name", "Log"},
-   {"args", json::obj{{"Message", Message.str()}}},
-   });
+  T->instant(Ctx, "Log", json::obj{{"Message", Message.str()}});
 }
 
-Span::Span(const Context , std::string Name) {
+Span::Span(const Context , llvm::StringRef Name) {
   if (!T)
 return;
-  // Clone the context, so that the original Context can be moved.
-  this->Ctx.emplace(Ctx.clone());
 
-  T->event(*this->Ctx, "B", json::obj{{"name", std::move(Name)}});
+  Callback = T->beginSpan(Ctx, Name);
+  if (!Callback)
+return;
+
   Args = llvm::make_unique();
 }
 
 Span::~Span() {
-  if (!T)
+  if (!Callback)
 return;
-  if (!Args)
-Args = llvm::make_unique();
-  T->event(*Ctx, "E",
-   Args ? json::obj{{"args", std::move(*Args)}} : json::obj{});
+
+  assert(Args && "Args must be non-null if Callback is defined");
+  Callback(std::move(*Args));
 }
 
 } // namespace trace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40489: [clangd] Changed tracing interfaces

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/Trace.cpp:134
+  Callback = T->beginSpan(Ctx, Name);
+  if (!Callback)
+return;

sammccall wrote:
> I'm not sure this is useful. Tracers that aren't interested in args can't opt 
> out by providing a null callback, unless they also don't care about the end 
> time.
> This seems unlikely (a null tracer, I guess)
One use-case that came to mind is turning the tracing on or off on per-request 
basis


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40489



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40489: [clangd] Changed tracing interfaces

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.



Comment at: clangd/Trace.cpp:134
+  Callback = T->beginSpan(Ctx, Name);
+  if (!Callback)
+return;

I'm not sure this is useful. Tracers that aren't interested in args can't opt 
out by providing a null callback, unless they also don't care about the end 
time.
This seems unlikely (a null tracer, I guess)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40489



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40489: [clangd] Changed tracing interfaces

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/Trace.h:39
+  /// Called when event with \p Name starts.
+  virtual void begin_event(const ContextData , llvm::StringRef Name) = 0;
+  /// Called when event with \p Name ends.

sammccall wrote:
> just call this begin?
> Otherwise style is `beginEvent` I think
The new name is `beginSpan`. Hope that sounds good.



Comment at: clangd/Trace.h:41
+  /// Called when event with \p Name ends.
+  virtual void end_event(const ContextData , llvm::StringRef Name,
+ json::obj &) = 0;

sammccall wrote:
> How is identity represented between begin/end event? via Name doesn't seem 
> robust, so why the need to pass it twice? Does providing different contexts 
> for start/end make sense?
> 
> It might be cleaner/more flexible to have
> 
> std::function begin()
> and call the return value to signal end.
> 
> This seems likely to be pretty easy for different providers to implement, and 
> is easy to use from Span.
Done. As discussed offline, the interface you propose seems much nicer.
Used names `beginSpan` and `instant`.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40489



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40489: [clangd] Changed tracing interfaces

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 126762.
ilya-biryukov marked 3 inline comments as done.
ilya-biryukov added a comment.

- Changed the EventTracer interface to return a callback from beginEvent 
instead of using begin_event/end_event method pairs.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40489

Files:
  clangd/Trace.cpp
  clangd/Trace.h

Index: clangd/Trace.h
===
--- clangd/Trace.h
+++ clangd/Trace.h
@@ -19,6 +19,7 @@
 #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TRACE_H_
 
 #include "Context.h"
+#include "Function.h"
 #include "JSONExpr.h"
 #include "llvm/ADT/Twine.h"
 #include "llvm/Support/raw_ostream.h"
@@ -28,12 +29,24 @@
 namespace trace {
 
 /// A consumer of trace events. The events are produced by Spans and trace::log.
+/// 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;
-  /// Consume a trace event.
-  virtual void event(const Context , llvm::StringRef Phase,
- json::obj &) = 0;
+
+  /// 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 ,
+ llvm::StringRef Name) = 0;
+
+  /// Called for instant events.
+  virtual void instant(const Context , llvm::StringRef Name,
+   json::obj &) = 0;
 };
 
 /// Sets up a global EventTracer that consumes events produced by Span and
@@ -50,9 +63,6 @@
 ///
 /// The format is documented here:
 /// https://docs.google.com/document/d/1CvAClvFfyA5R-PhYUmn5OOQtYMH4h6I0nSsKchNAySU/preview
-///
-/// The implementation supports concurrent calls and can be used as a global
-/// tracer (i.e., can be put into a global Context).
 std::unique_ptr createJSONTracer(llvm::raw_ostream ,
   bool Pretty = false);
 
@@ -67,16 +77,16 @@
 /// SomeJSONExpr is evaluated and copied only if actually needed.
 class Span {
 public:
-  Span(const Context , std::string Name);
+  Span(const Context , llvm::StringRef Name);
   ~Span();
 
   /// Returns mutable span metadata if this span is interested.
   /// Prefer to use SPAN_ATTACH rather than accessing this directly.
   json::obj *args() { return Args.get(); }
 
 private:
-  llvm::Optional Ctx;
   std::unique_ptr Args;
+  EventTracer::EndEventCallback Callback;
 };
 
 #define SPAN_ATTACH(S, Name, Expr) \
Index: clangd/Trace.cpp
===
--- clangd/Trace.cpp
+++ clangd/Trace.cpp
@@ -44,10 +44,24 @@
 Out.flush();
   }
 
+  EndEventCallback beginSpan(const Context ,
+ llvm::StringRef Name) override {
+jsonEvent("B", json::obj{{"name", Name}});
+
+// The callback that will run when event ends.
+return [this](json::Expr &) {
+  jsonEvent("E", json::obj{{"args", std::move(Args)}});
+};
+  }
+
+  void instant(const Context , llvm::StringRef Name,
+   json::obj &) override {
+jsonEvent("i", json::obj{{"name", Name}, {"args", std::move(Args)}});
+  }
+
   // Record an event on the current thread. ph, pid, tid, ts are set.
   // Contents must be a list of the other JSON key/values.
-  void event(const Context , StringRef Phase,
- json::obj &) override {
+  void jsonEvent(StringRef Phase, json::obj &) {
 uint64_t TID = get_threadid();
 std::lock_guard Lock(Mu);
 // If we haven't already, emit metadata describing this thread.
@@ -109,30 +123,26 @@
 void log(const Context , const Twine ) {
   if (!T)
 return;
-  T->event(Ctx, "i",
-   json::obj{
-   {"name", "Log"},
-   {"args", json::obj{{"Message", Message.str()}}},
-   });
+  T->instant(Ctx, "Log", json::obj{{"Message", Message.str()}});
 }
 
-Span::Span(const Context , std::string Name) {
+Span::Span(const Context , llvm::StringRef Name) {
   if (!T)
 return;
-  // Clone the context, so that the original Context can be moved.
-  this->Ctx.emplace(Ctx.clone());
 
-  T->event(*this->Ctx, "B", json::obj{{"name", std::move(Name)}});
+  Callback = T->beginSpan(Ctx, Name);
+  if (!Callback)
+return;
+
   Args = llvm::make_unique();
 }
 
 Span::~Span() {
-  if (!T)
+  if (!Callback)
 return;
-  if (!Args)
-Args = llvm::make_unique();
-  T->event(*Ctx, "E",
-   Args ? json::obj{{"args", std::move(*Args)}} : json::obj{});
+
+  assert(Args && "Args must be non-null if Callback is defined");
+  Callback(std::move(*Args));
 }
 
 } // namespace trace

[PATCH] D40489: [clangd] Changed tracing interfaces

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 126329.
ilya-biryukov added a comment.

Updated the patch after changes to Context


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40489

Files:
  clangd/Trace.cpp
  clangd/Trace.h

Index: clangd/Trace.h
===
--- clangd/Trace.h
+++ clangd/Trace.h
@@ -28,12 +28,21 @@
 namespace trace {
 
 /// A consumer of trace events. The events are produced by Spans and trace::log.
+/// Calls to begin_event and end_event with the same name are always properly
+/// nested by using a RAII object (Span).
+/// Implmentations of this interface must be thread-safe.
 class EventTracer {
 public:
   virtual ~EventTracer() = default;
-  /// Consume a trace event.
-  virtual void event(const Context , llvm::StringRef Phase,
- json::obj &) = 0;
+
+  /// Called when event with \p Name starts.
+  virtual void begin_event(const Context , llvm::StringRef Name) = 0;
+  /// Called when event with \p Name ends.
+  virtual void end_event(const Context , llvm::StringRef Name,
+ json::obj &) = 0;
+  /// Called for instant events.
+  virtual void instant_event(const Context , llvm::StringRef Name,
+ json::obj &) = 0;
 };
 
 /// Sets up a global EventTracer that consumes events produced by Span and
@@ -67,15 +76,16 @@
 /// SomeJSONExpr is evaluated and copied only if actually needed.
 class Span {
 public:
-  Span(const Context , std::string Name);
+  Span(const Context , llvm::StringRef Name);
   ~Span();
 
   /// Returns mutable span metadata if this span is interested.
   /// Prefer to use SPAN_ATTACH rather than accessing this directly.
   json::obj *args() { return Args.get(); }
 
 private:
   llvm::Optional Ctx;
+  std::string Name;
   std::unique_ptr Args;
 };
 
Index: clangd/Trace.cpp
===
--- clangd/Trace.cpp
+++ clangd/Trace.cpp
@@ -44,10 +44,23 @@
 Out.flush();
   }
 
+  void begin_event(const Context , llvm::StringRef Name) override {
+jsonEvent("B", json::obj{{"name", Name}});
+  }
+
+  void end_event(const Context , llvm::StringRef Name,
+ json::obj &) override {
+jsonEvent("E", json::obj{{"args", std::move(Args)}});
+  }
+
+  void instant_event(const Context , llvm::StringRef Name,
+ json::obj &) override {
+jsonEvent("i", json::obj{{"name", Name}, {"args", std::move(Args)}});
+  }
+
   // Record an event on the current thread. ph, pid, tid, ts are set.
   // Contents must be a list of the other JSON key/values.
-  void event(const Context , StringRef Phase,
- json::obj &) override {
+  void jsonEvent(StringRef Phase, json::obj &) {
 uint64_t TID = get_threadid();
 std::lock_guard Lock(Mu);
 // If we haven't already, emit metadata describing this thread.
@@ -109,30 +122,24 @@
 void log(const Context , const Twine ) {
   if (!T)
 return;
-  T->event(Ctx, "i",
-   json::obj{
-   {"name", "Log"},
-   {"args", json::obj{{"Message", Message.str()}}},
-   });
+  T->instant_event(Ctx, "Log", json::obj{{"Message", Message.str()}});
 }
 
-Span::Span(const Context , std::string Name) {
+Span::Span(const Context , llvm::StringRef Name) : Name(Name) {
   if (!T)
 return;
   // Clone the context, so that the original Context can be moved.
   this->Ctx.emplace(Ctx.clone());
 
-  T->event(*this->Ctx, "B", json::obj{{"name", std::move(Name)}});
+  T->begin_event(*this->Ctx, this->Name);
   Args = llvm::make_unique();
 }
 
 Span::~Span() {
   if (!T)
 return;
-  if (!Args)
-Args = llvm::make_unique();
-  T->event(*Ctx, "E",
-   Args ? json::obj{{"args", std::move(*Args)}} : json::obj{});
+  assert(Args && "Args can't be null at this point");
+  T->end_event(*this->Ctx, Name, std::move(*Args));
 }
 
 } // namespace trace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40489: [clangd] Changed tracing interfaces

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 126318.
ilya-biryukov added a comment.

- Update the patch to accomodate changes from tracing and Context patches.
- Updated tracing after changes to Context. We now store a clone() of Context 
in Span instead of ContextData.
- Pass Context by const-ref instead of by ref.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40489

Files:
  clangd/JSONRPCDispatcher.cpp
  clangd/Trace.cpp
  clangd/Trace.h

Index: clangd/Trace.h
===
--- clangd/Trace.h
+++ clangd/Trace.h
@@ -28,12 +28,21 @@
 namespace trace {
 
 /// A consumer of trace events. The events are produced by Spans and trace::log.
+/// Calls to begin_event and end_event with the same name are always properly
+/// nested by using a RAII object (Span).
+/// Implmentations of this interface must be thread-safe.
 class EventTracer {
 public:
   virtual ~EventTracer() = default;
-  /// Consume a trace event.
-  virtual void event(const ContextData , llvm::StringRef Phase,
- json::obj &) = 0;
+
+  /// Called when event with \p Name starts.
+  virtual void begin_event(const Context , llvm::StringRef Name) = 0;
+  /// Called when event with \p Name ends.
+  virtual void end_event(const Context , llvm::StringRef Name,
+ json::obj &) = 0;
+  /// Called for instant events.
+  virtual void instant_event(const Context , llvm::StringRef Name,
+ json::obj &) = 0;
 };
 
 /// Sets up a global EventTracer that consumes events produced by Span and
@@ -57,7 +66,7 @@
   bool Pretty = false);
 
 /// Records a single instant event, associated with the current thread.
-void log(Context , const llvm::Twine );
+void log(const Context , const llvm::Twine );
 
 /// Records an event whose duration is the lifetime of the Span object.
 /// This is the main public interface for producing tracing events.
@@ -67,15 +76,16 @@
 /// SomeJSONExpr is evaluated and copied only if actually needed.
 class Span {
 public:
-  Span(Context , std::string Name);
+  Span(const Context , llvm::StringRef Name);
   ~Span();
 
   /// Returns mutable span metadata if this span is interested.
   /// Prefer to use SPAN_ATTACH rather than accessing this directly.
   json::obj *args() { return Args.get(); }
 
 private:
-  const ContextData 
+  llvm::Optional Ctx;
+  std::string Name;
   std::unique_ptr Args;
 };
 
Index: clangd/Trace.cpp
===
--- clangd/Trace.cpp
+++ clangd/Trace.cpp
@@ -44,10 +44,23 @@
 Out.flush();
   }
 
+  void begin_event(const Context , llvm::StringRef Name) override {
+jsonEvent("B", json::obj{{"name", Name}});
+  }
+
+  void end_event(const Context , llvm::StringRef Name,
+ json::obj &) override {
+jsonEvent("E", json::obj{{"args", std::move(Args)}});
+  }
+
+  void instant_event(const Context , llvm::StringRef Name,
+ json::obj &) override {
+jsonEvent("i", json::obj{{"name", Name}, {"args", std::move(Args)}});
+  }
+
   // Record an event on the current thread. ph, pid, tid, ts are set.
   // Contents must be a list of the other JSON key/values.
-  void event(const ContextData , StringRef Phase,
- json::obj &) override {
+  void jsonEvent(StringRef Phase, json::obj &) {
 uint64_t TID = get_threadid();
 std::lock_guard Lock(Mu);
 // If we haven't already, emit metadata describing this thread.
@@ -106,30 +119,27 @@
   return llvm::make_unique(OS, Pretty);
 }
 
-void log(Context , const Twine ) {
+void log(const Context , const Twine ) {
   if (!T)
 return;
-  T->event(*Ctx, "i",
-   json::obj{
-   {"name", "Log"},
-   {"args", json::obj{{"Message", Message.str()}}},
-   });
+  T->instant_event(Ctx, "Log", json::obj{{"Message", Message.str()}});
 }
 
-Span::Span(Context , std::string Name) : Ctx(*Ctx) {
+Span::Span(const Context , llvm::StringRef Name) : Name(Name) {
   if (!T)
 return;
-  T->event(this->Ctx, "B", json::obj{{"name", std::move(Name)}});
+  // Clone the context, so that the original Context can be moved.
+  this->Ctx.emplace(Ctx.clone());
+
+  T->begin_event(*this->Ctx, this->Name);
   Args = llvm::make_unique();
 }
 
 Span::~Span() {
   if (!T)
 return;
-  if (!Args)
-Args = llvm::make_unique();
-  T->event(Ctx, "E",
-   Args ? json::obj{{"args", std::move(*Args)}} : json::obj{});
+  assert(Args && "Args can't be null at this point");
+  T->end_event(*this->Ctx, Name, std::move(*Args));
 }
 
 } // namespace trace
Index: clangd/JSONRPCDispatcher.cpp
===
--- clangd/JSONRPCDispatcher.cpp
+++ clangd/JSONRPCDispatcher.cpp
@@ -60,14 +60,14 @@
 }
 
 void clangd::reply(const Context , json::Expr &) {
-  auto ID = Ctx->get(IDKey);
+  auto ID = Ctx.get(IDKey);
   if (!ID) {
 

[PATCH] D40489: [clangd] Changed tracing interfaces

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clangd/Trace.cpp:138
 return;
-  if (!Args)
-Args = llvm::make_unique();
-  T->event(Ctx, "E",
-   Args ? json::obj{{"args", std::move(*Args)}} : json::obj{});
+  assert(Args && "Args can't be null at this point");
+  T->end_event(Ctx, Name, std::move(*Args));

ilya-biryukov wrote:
> sammccall wrote:
> > why not?
> Because `T` must outlive the `Span`, so we can't really have `if (!T)` 
> evaluate to different things in constructor and destructor.
> Am I missing something?
I was being disingenuous, I agree with you.
But can you change the message to "Changed tracer during a span"?

assert(Args) and "Args can't be null at this point" are basically synonyms :-)


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40489



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40489: [clangd] Changed tracing interfaces

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clangd/Trace.cpp:138
 return;
-  if (!Args)
-Args = llvm::make_unique();
-  T->event(Ctx, "E",
-   Args ? json::obj{{"args", std::move(*Args)}} : json::obj{});
+  assert(Args && "Args can't be null at this point");
+  T->end_event(Ctx, Name, std::move(*Args));

sammccall wrote:
> why not?
Because `T` must outlive the `Span`, so we can't really have `if (!T)` evaluate 
to different things in constructor and destructor.
Am I missing something?


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40489



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40489: [clangd] Changed tracing interfaces

[Sam McCall via Phabricator via cfe-commits]

sammccall added inline comments.



Comment at: clangd/Trace.cpp:138
 return;
-  if (!Args)
-Args = llvm::make_unique();
-  T->event(Ctx, "E",
-   Args ? json::obj{{"args", std::move(*Args)}} : json::obj{});
+  assert(Args && "Args can't be null at this point");
+  T->end_event(Ctx, Name, std::move(*Args));

why not?



Comment at: clangd/Trace.h:39
+  /// Called when event with \p Name starts.
+  virtual void begin_event(const ContextData , llvm::StringRef Name) = 0;
+  /// Called when event with \p Name ends.

just call this begin?
Otherwise style is `beginEvent` I think



Comment at: clangd/Trace.h:41
+  /// Called when event with \p Name ends.
+  virtual void end_event(const ContextData , llvm::StringRef Name,
+ json::obj &) = 0;

How is identity represented between begin/end event? via Name doesn't seem 
robust, so why the need to pass it twice? Does providing different contexts for 
start/end make sense?

It might be cleaner/more flexible to have

std::function begin()
and call the return value to signal end.

This seems likely to be pretty easy for different providers to implement, and 
is easy to use from Span.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40489



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40489: [clangd] Changed tracing interfaces

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 125696.
ilya-biryukov added a comment.
Herald added a subscriber: klimek.

- Update the patch to accomodate changes from tracing and Context patches.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D40489

Files:
  clangd/Trace.cpp
  clangd/Trace.h

Index: clangd/Trace.h
===
--- clangd/Trace.h
+++ clangd/Trace.h
@@ -28,12 +28,21 @@
 namespace trace {
 
 /// A consumer of trace events. The events are produced by Spans and trace::log.
+/// Calls to begin_event and end_event with the same name are always properly
+/// nested by using a RAII object (Span).
+/// Implmentations of this interface must be thread-safe.
 class EventTracer {
 public:
   virtual ~EventTracer() = default;
-  /// Consume a trace event.
-  virtual void event(const ContextData , llvm::StringRef Phase,
- json::obj &) = 0;
+
+  /// Called when event with \p Name starts.
+  virtual void begin_event(const ContextData , llvm::StringRef Name) = 0;
+  /// Called when event with \p Name ends.
+  virtual void end_event(const ContextData , llvm::StringRef Name,
+ json::obj &) = 0;
+  /// Called for instant events.
+  virtual void instant_event(const ContextData , llvm::StringRef Name,
+ json::obj &) = 0;
 };
 
 /// Sets up a global EventTracer that consumes events produced by Span and
@@ -67,15 +76,16 @@
 /// SomeJSONExpr is evaluated and copied only if actually needed.
 class Span {
 public:
-  Span(Context , std::string Name);
+  Span(Context , llvm::StringRef Name);
   ~Span();
 
   /// Returns mutable span metadata if this span is interested.
   /// Prefer to use SPAN_ATTACH rather than accessing this directly.
   json::obj *args() { return Args.get(); }
 
 private:
   const ContextData 
+  std::string Name;
   std::unique_ptr Args;
 };
 
Index: clangd/Trace.cpp
===
--- clangd/Trace.cpp
+++ clangd/Trace.cpp
@@ -44,10 +44,23 @@
 Out.flush();
   }
 
+  void begin_event(const ContextData , llvm::StringRef Name) override {
+jsonEvent("B", json::obj{{"name", Name}});
+  }
+
+  void end_event(const ContextData , llvm::StringRef Name,
+ json::obj &) override {
+jsonEvent("E", json::obj{{"args", std::move(Args)}});
+  }
+
+  void instant_event(const ContextData , llvm::StringRef Name,
+ json::obj &) override {
+jsonEvent("i", json::obj{{"name", Name}, {"args", std::move(Args)}});
+  }
+
   // Record an event on the current thread. ph, pid, tid, ts are set.
   // Contents must be a list of the other JSON key/values.
-  void event(const ContextData , StringRef Phase,
- json::obj &) override {
+  void jsonEvent(StringRef Phase, json::obj &) {
 uint64_t TID = get_threadid();
 std::lock_guard Lock(Mu);
 // If we haven't already, emit metadata describing this thread.
@@ -109,27 +122,21 @@
 void log(Context , const Twine ) {
   if (!T)
 return;
-  T->event(*Ctx, "i",
-   json::obj{
-   {"name", "Log"},
-   {"args", json::obj{{"Message", Message.str()}}},
-   });
+  T->instant_event(*Ctx, "Log", json::obj{{"Message", Message.str()}});
 }
 
-Span::Span(Context , std::string Name) : Ctx(*Ctx) {
+Span::Span(Context , llvm::StringRef Name) : Ctx(*Ctx), Name(Name) {
   if (!T)
 return;
-  T->event(this->Ctx, "B", json::obj{{"name", std::move(Name)}});
+  T->begin_event(this->Ctx, this->Name);
   Args = llvm::make_unique();
 }
 
 Span::~Span() {
   if (!T)
 return;
-  if (!Args)
-Args = llvm::make_unique();
-  T->event(Ctx, "E",
-   Args ? json::obj{{"args", std::move(*Args)}} : json::obj{});
+  assert(Args && "Args can't be null at this point");
+  T->end_event(Ctx, Name, std::move(*Args));
 }
 
 } // namespace trace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40489: [clangd] Changed tracing interfaces

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov planned changes to this revision.
ilya-biryukov added a comment.

Have to update to match the new `Context` implementation.


https://reviews.llvm.org/D40489



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40489: [clangd] Changed tracing interfaces

[Pavel Sychev via Phabricator via cfe-commits]

luckygeck added inline comments.



Comment at: clangd/Trace.h:41
+  /// Context.
   static PtrKey CtxKey;
 

ilya-biryukov wrote:
> luckygeck wrote:
> > This is a (1)static non-pod member in an (2) interface. Is it really a good 
> > idea? If we plan to have only one ctxkey, then maybe let's make it not 
> > bound to EventTracer?
> It does not have any data members, has trivial default constructor and 
> trivial destructor.
> I don't think there are any problems we're gonna hit with this one, or am I 
> missing something?
> 
> > then maybe let's make it not bound to EventTracer?
> Do you propose to move it out of the `class` into the `clangd::trace` 
> namespace instead?
I've just looked through the change adding Context and Key<>\PtrKey<> thingy. 
Yes, these classes are POD, so it is fine to have them static  (as you rely 
only on their address). 

Yes, I'd prefer to move the key out of `class` into a namespace - interface 
looks better without any data IMO.



Comment at: clangd/Trace.h:49
+  virtual void end_event(Context , llvm::StringRef Name,
+ json::obj &) = 0;
+  /// Called for instant events.

ilya-biryukov wrote:
> luckygeck wrote:
> > Maybe it is better to calculate these Args only if the tracing is enabled? 
> > This might be done at least this two ways:
> > 1) Have `bool is_tracing_enabled()` method that we can check before 
> > calculating args.
> > 2) Pass a function that will produce json::obj when called.
> The current implementation won't compute args if `EventTracer` inside a 
> `Context` is null, so I think this should cover our needs for per-request 
> tracing (see `SPAN_ATTACH` macro).
> But `is_tracing_enabled()` makes sense if we'd like to turn the tracing off 
> in a middle of a single `Context` lifetime. This would make sense for global 
> tracer, is this the use-case you anticipate?
> 
Oh, ok - setting tracer in a context only when we need to looks good enough.


https://reviews.llvm.org/D40489



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40489: [clangd] Changed tracing interfaces

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov marked an inline comment as done.
ilya-biryukov added inline comments.



Comment at: clangd/Trace.h:41
+  /// Context.
   static PtrKey CtxKey;
 

luckygeck wrote:
> This is a (1)static non-pod member in an (2) interface. Is it really a good 
> idea? If we plan to have only one ctxkey, then maybe let's make it not bound 
> to EventTracer?
It does not have any data members, has trivial default constructor and trivial 
destructor.
I don't think there are any problems we're gonna hit with this one, or am I 
missing something?

> then maybe let's make it not bound to EventTracer?
Do you propose to move it out of the `class` into the `clangd::trace` namespace 
instead?



Comment at: clangd/Trace.h:49
+  virtual void end_event(Context , llvm::StringRef Name,
+ json::obj &) = 0;
+  /// Called for instant events.

luckygeck wrote:
> Maybe it is better to calculate these Args only if the tracing is enabled? 
> This might be done at least this two ways:
> 1) Have `bool is_tracing_enabled()` method that we can check before 
> calculating args.
> 2) Pass a function that will produce json::obj when called.
The current implementation won't compute args if `EventTracer` inside a 
`Context` is null, so I think this should cover our needs for per-request 
tracing (see `SPAN_ATTACH` macro).
But `is_tracing_enabled()` makes sense if we'd like to turn the tracing off in 
a middle of a single `Context` lifetime. This would make sense for global 
tracer, is this the use-case you anticipate?



https://reviews.llvm.org/D40489



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40489: [clangd] Changed tracing interfaces

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 124371.
ilya-biryukov added a comment.

- Fixed a typo.


https://reviews.llvm.org/D40489

Files:
  clangd/Trace.cpp
  clangd/Trace.h

Index: clangd/Trace.h
===
--- clangd/Trace.h
+++ clangd/Trace.h
@@ -27,12 +27,29 @@
 namespace clangd {
 namespace trace {
 
+/// A sink for consuming tracing events. Calls to begin_event and end_event with
+/// the same name are always properly nested by using a RAII object (Span).
+/// However, global tracers (i.e., returned from the global Context object) may
+/// be called concurrently and have to be properly synchronized. For per-request
+/// EventTracers (i.e., returned from the request-specific Context objects) the
+/// calls to begin_event and end_event must be properly synchronized and must
+/// never be called concurrently.
 class EventTracer {
 public:
+  /// A key used by trace::Span and trace::log to find EventTracer in a \p
+  /// Context.
   static PtrKey CtxKey;
 
   virtual ~EventTracer() = default;
-  virtual void event(llvm::StringRef Phase, json::obj &) = 0;
+
+  /// Called when event with \p Name starts.
+  virtual void begin_event(Context , llvm::StringRef Name) = 0;
+  /// Called when event with \p Name ends.
+  virtual void end_event(Context , llvm::StringRef Name,
+ json::obj &) = 0;
+  /// Called for instant events.
+  virtual void instant_event(Context , llvm::StringRef Name,
+ json::obj &) = 0;
 };
 
 /// Create an instance of EventTracer that produces an output in the Trace Event
@@ -66,6 +83,7 @@
 
 private:
   Context 
+  std::string Name;
   std::unique_ptr Args;
 };
 
Index: clangd/Trace.cpp
===
--- clangd/Trace.cpp
+++ clangd/Trace.cpp
@@ -45,9 +45,23 @@
 Out.flush();
   }
 
+  void begin_event(Context , llvm::StringRef Name) override {
+jsonEvent("B", json::obj{{"name", Name}});
+  }
+
+  void end_event(Context , llvm::StringRef Name,
+ json::obj &) override {
+jsonEvent("E", json::obj{{"name", Name}, {"args", std::move(Args)}});
+  }
+
+  void instant_event(Context , llvm::StringRef Name,
+ json::obj &) override {
+jsonEvent("i", json::obj{{"name", Name}, {"args", std::move(Args)}});
+  }
+
   // Record an event on the current thread. ph, pid, tid, ts are set.
   // Contents must be a list of the other JSON key/values.
-  void event(StringRef Phase, json::obj &) override {
+  void jsonEvent(StringRef Phase, json::obj &) {
 uint64_t TID = get_threadid();
 std::lock_guard Lock(Mu);
 // If we haven't already, emit metadata describing this thread.
@@ -103,27 +117,23 @@
   EventTracer *T = Ctx.get(EventTracer::CtxKey);
   if (!T)
 return;
-  T->event("i", json::obj{
-{"name", "Log"},
-{"args", json::obj{{"Message", Message.str()}}},
-});
+  T->instant_event("Log", json::obj{{"Message", Message.str()}});
 }
 
-Span::Span(Context , std::string Name) : Ctx(Ctx) {
-  EventTracer* T = Ctx.get(EventTracer::CtxKey);
+Span::Span(Context , std::string Name) : Ctx(Ctx), Name(std::move(Name)) {
+  EventTracer *T = Ctx.get(EventTracer::CtxKey);
   if (!T)
 return;
-  T->event("B", json::obj{{"name", std::move(Name)}});
+  T->begin_event(this->Name);
   Args = llvm::make_unique();
 }
 
 Span::~Span() {
   auto T = Ctx.get(EventTracer::CtxKey);
   if (!T)
 return;
-  if (!Args)
-Args = llvm::make_unique();
-  T->event("E", Args ? json::obj{{"args", std::move(*Args)}} : json::obj{});
+  assert(Args && "Args can't be null at this point");
+  T->end_event(Name, std::move(*Args));
 }
 
 } // namespace trace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40489: [clangd] Changed tracing interfaces

[Pavel Sychev via Phabricator via cfe-commits]

luckygeck added inline comments.



Comment at: clangd/Trace.h:41
+  /// Context.
   static PtrKey CtxKey;
 

This is a (1)static non-pod member in an (2) interface. Is it really a good 
idea? If we plan to have only one ctxkey, then maybe let's make it not bound to 
EventTracer?



Comment at: clangd/Trace.h:45
+
+  /// Called when event with \p Name stars.
+  virtual void begin_event(Context , llvm::StringRef Name) = 0;

s/stars/starts.



Comment at: clangd/Trace.h:49
+  virtual void end_event(Context , llvm::StringRef Name,
+ json::obj &) = 0;
+  /// Called for instant events.

Maybe it is better to calculate these Args only if the tracing is enabled? This 
might be done at least this two ways:
1) Have `bool is_tracing_enabled()` method that we can check before calculating 
args.
2) Pass a function that will produce json::obj when called.


https://reviews.llvm.org/D40489



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40489: [clangd] Changed tracing interfaces

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.

Introduced begin_event, end_event and instant_event instead of a
string phase name.


https://reviews.llvm.org/D40489

Files:
  clangd/Trace.cpp
  clangd/Trace.h

Index: clangd/Trace.h
===
--- clangd/Trace.h
+++ clangd/Trace.h
@@ -27,12 +27,29 @@
 namespace clangd {
 namespace trace {
 
+/// A sink for consuming tracing events. Calls to begin_event and end_event with
+/// the same name are always properly nested by using a RAII object (Span).
+/// However, global tracers (i.e., returned from the global Context object) may
+/// be called concurrently and have to be properly synchronized. For per-request
+/// EventTracers (i.e., returned from the request-specific Context objects) the
+/// calls to begin_event and end_event must be properly synchronized and must
+/// never be called concurrently.
 class EventTracer {
 public:
+  /// A key used by trace::Span and trace::log to find EventTracer in a \p
+  /// Context.
   static PtrKey CtxKey;
 
   virtual ~EventTracer() = default;
-  virtual void event(llvm::StringRef Phase, json::obj &) = 0;
+
+  /// Called when event with \p Name stars.
+  virtual void begin_event(Context , llvm::StringRef Name) = 0;
+  /// Called when event with \p Name ends.
+  virtual void end_event(Context , llvm::StringRef Name,
+ json::obj &) = 0;
+  /// Called for instant events.
+  virtual void instant_event(Context , llvm::StringRef Name,
+ json::obj &) = 0;
 };
 
 /// Create an instance of EventTracer that produces an output in the Trace Event
@@ -66,6 +83,7 @@
 
 private:
   Context 
+  std::string Name;
   std::unique_ptr Args;
 };
 
Index: clangd/Trace.cpp
===
--- clangd/Trace.cpp
+++ clangd/Trace.cpp
@@ -45,9 +45,23 @@
 Out.flush();
   }
 
+  void begin_event(Context , llvm::StringRef Name) override {
+jsonEvent("B", json::obj{{"name", Name}});
+  }
+
+  void end_event(Context , llvm::StringRef Name,
+ json::obj &) override {
+jsonEvent("E", json::obj{{"name", Name}, {"args", std::move(Args)}});
+  }
+
+  void instant_event(Context , llvm::StringRef Name,
+ json::obj &) override {
+jsonEvent("i", json::obj{{"name", Name}, {"args", std::move(Args)}});
+  }
+
   // Record an event on the current thread. ph, pid, tid, ts are set.
   // Contents must be a list of the other JSON key/values.
-  void event(StringRef Phase, json::obj &) override {
+  void jsonEvent(StringRef Phase, json::obj &) {
 uint64_t TID = get_threadid();
 std::lock_guard Lock(Mu);
 // If we haven't already, emit metadata describing this thread.
@@ -103,27 +117,23 @@
   EventTracer *T = Ctx.get(EventTracer::CtxKey);
   if (!T)
 return;
-  T->event("i", json::obj{
-{"name", "Log"},
-{"args", json::obj{{"Message", Message.str()}}},
-});
+  T->instant_event("Log", json::obj{{"Message", Message.str()}});
 }
 
-Span::Span(Context , std::string Name) : Ctx(Ctx) {
-  EventTracer* T = Ctx.get(EventTracer::CtxKey);
+Span::Span(Context , std::string Name) : Ctx(Ctx), Name(std::move(Name)) {
+  EventTracer *T = Ctx.get(EventTracer::CtxKey);
   if (!T)
 return;
-  T->event("B", json::obj{{"name", std::move(Name)}});
+  T->begin_event(this->Name);
   Args = llvm::make_unique();
 }
 
 Span::~Span() {
   auto T = Ctx.get(EventTracer::CtxKey);
   if (!T)
 return;
-  if (!Args)
-Args = llvm::make_unique();
-  T->event("E", Args ? json::obj{{"args", std::move(*Args)}} : json::obj{});
+  assert(Args && "Args can't be null at this point");
+  T->end_event(Name, std::move(*Args));
 }
 
 } // namespace trace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits