Event logging implementation for the Chrome Trace Event format. about vulkan-performance-layers HOT 6 CLOSED

Thanks for writing this down:

Current Event class does not have a "phase" and putting the phase as an attribute might mess things up because we have to change the other loggers to ignore it. I suggest the following code:

IMO we shouldn't discard this solution prematurely. I find it natural for loggers to care about some attributes and discard the rest.

Similarly, having an explicit phase attribute could inform the TraceLogger to log some events and skip the others.

std::string TraceEventToStr(TraceEvent *event)

It's unclear to me how this would work with the existing implementation. We have no mechanism to tell if an event is a TraceEvent or not. Although it shouldn't be too hard to to add this.

from vulkan-performance-layers.

Similarly, having an explicit phase attribute could inform the TraceLogger to log some events and skip the others.

For this, I was thinking of using the filter logger and filtering them based on the log level.

from vulkan-performance-layers.

It's unclear to me how this would work with the existing implementation. We have no mechanism to tell if an event is a TraceEvent or not. Although it shouldn't be too hard to to add this.

Currently, the events in the compile time and runtime layer are extended from Event. Instead we can extend them from TraceEvent. Those layers will have a LayerDataWithTraceLogger which accepts a TraceEvent, converts it to a string using this function and logs it.
Isn't it gonna be a compile error if we pass an event which is not a TraceEvent to this function?

from vulkan-performance-layers.

Similarly, having an explicit phase attribute could inform the TraceLogger to log some events and skip the others.

For this, I was thinking of using the filter logger and filtering them based on the log level.

Currently, the events in the compile time and runtime layer are extended from Event. Instead we can extend them from TraceEvent. Those layers will have a LayerDataWithTraceLogger which accepts a TraceEvent, converts it to a string using this function and logs it.

I assumed we wanted to maintain that the same event class can be logged by all loggers (modulo ones that don't care about an event), and that logging happens when we call AddEvent(Event *). And in general I don't think we should duplicate event types (e.g., having 2 shader creation events, one for CSV/Common and one for Trace).

If you add a new base event type that can only be passed to TraceLogger, you would also have to teach to extend the whole logger hierarchy (broadcast, filter, and test code).

from vulkan-performance-layers.

Similarly, having an explicit phase attribute could inform the TraceLogger to log some events and skip the others.

For this, I was thinking of using the filter logger and filtering them based on the log level.

Currently, the events in the compile time and runtime layer are extended from Event. Instead we can extend them from TraceEvent. Those layers will have a LayerDataWithTraceLogger which accepts a TraceEvent, converts it to a string using this function and logs it.

I assumed we wanted to maintain that the same event class can be logged by all loggers (modulo ones that don't care about an event), and that logging happens when we call AddEvent(Event *). And in general I don't think we should duplicate event types (e.g., having 2 shader creation events, one for CSV/Common and one for Trace).

If you add a new base event type that can only be passed to TraceLogger, you would also have to teach to extend the whole logger hierarchy (broadcast, filter, and test code).

Yeah. You're right. I'll re-think about it.

from vulkan-performance-layers.

We want these two types of events:

{ "name": "Complete Event", "cat": "pipeline", "ph": "X", "ts": 12345, "pid": 123, "tid": 456, "dur": 123,  "args":  { "someArg": [1, 2]} }

{ "name": "Instant Event", "cat": "start event", "ph": "i", "ts": 12345, "pid": 123, "tid": 456, "s": "g",  "args":  {} }

We can have a compound attribute constructed from our current set of primitive attributes and used by the TraceEventLogger and ignored by other loggers. To support Instant Events and Complete Events the compound event has to have cateogry, phase, pid, and tid. We also might want to consider having a vector of attributes for the optional attributes (e.g., shader hashes) or the ones that might be required based on the event type (e.g., duration or scope).
All events have a timestamp attribute which shows the end of an event. The visualizer associates the timestamp with the beginning of the event so the Complete Events have to calculate the start timestamp by subtracting duration from the event's timestamp. Hence, the helper function that takes the attribute and converts it to a string (e.g., TraceAttrToString()), should also receive the timestamp from the caller.

The compound attribute could be like this:

class TraceEventAttribute : public Attribute {
 public:
  static constexpr ValueType id_ = ValueType::kTraceEvent;

  TraceEventAttribute(const char *name, const char *cat,
                      const char *phase, int64_t pid,
                      int64_t tid, std::initializer_list<Attribute *> args)
      : Attribute(name, ValueType::kTraceEvent),
        category_("cat", cat),
        phase_("ph", phase),
        pid_("pid", pid),
        tid_("tid", tid)
  {
    args_ = {args.begin(), args.end()};
  }

 // Getter methods

 private:
  StringAttr category_;
  StringAttr phase_;
  Int64Attr pid_;
  Int64Attr tid_;
  std::vector<Attribute *> args_;
};

Each event scans the args vector based on phase to extract the arguments it wants. Since args is small, scanning it shouldn't have much overhead.

from vulkan-performance-layers.