Implemented color coding and Vertex labels in XRay Graph

[varno]

Summary

A patch to enable the llvm-xray graph subcommand to color edges and
vertices based on statistics and to annotate vertices with statistics.

Depends on D27243

[varno]

• Fixing Test with changes added to handle color.

[dblaikie]

Needs test coverage for the color functionality, I think? (I don't see any colors being tested in the tests)

tools/llvm-xray/xray-graph.cc

264–270

Could use a member pointer & a helper function (possibly template) to maybe make these shorter/more reliable (by only having to mention the member name once - less chance of accidentally mismatching them).

310

Typo, 'function'

Also probably s/Normalise Statistics/normaliseStatistics/

Also, normalize rather than normalise (consistency with the rest of the codebase)

485–508

Could use another member pointer helper to handle this table, if you like.

[dberris]

tools/llvm-xray/xray-graph.cc

439–443

Probably missing a few punctuation marks. I think some structured fomulae might help with what exactly it's doing, like:

// Evaluates a polynomial given the provided range of coefficients in order.
//
//    p(x) = a[0]*x^0 + a[1]*x^1 + a[2]*x^2 + ... + a[n]*x^n
//
// We use Horner's Method for numerical stability for mapping to an 8-bit
// fixed point domain.

460–468

You probably want to document how you came up with these polynomials, even potentially citing some sources on how one might possibly start changing these.

471–473

Reading this, I think you're better off making polyEval take an llvm::ArrayRef instead (to make it easier to read).

510

Can you document why compare is returning the square root? You mentioned this offline in our discussion, but it would be good to document the reasons here.

[varno]

• Clang format

[varno]

Fixed and addressed comments.

[varno]

tools/llvm-xray/xray-graph.cc

264–270

Discussion resulted in this comment being seen as not easy to do due to specifics.

[dberris]

In general I think this could do with a bit more documentation, about the semantics of what we're trying to do (as opposed to how we're doing it).

tools/llvm-xray/xray-graph.cc

170

Empty line after this?

171

This is a template, no need to be static (it's implicitly inline).

175

Style guide says identifiers ought to have an uppercase start letter.

184–187

Can we describe this in less round-about way?

We can spend a few words in documentation to indicate what the side-effects are and some detail of what the algorithm being implemented is.

192–193

Was the intent to use llvm::Expected<...>? In which case I'd prefer you change this now.

[dberris]

Also, I think you'd want to rebase this given that llvm-xray graph has landed upstream.

[varno]

• Clang format
• Fixed tests after rebase
• Addressed review comments

[varno]

Also has been rebased to master.

[dberris]

tools/llvm-xray/xray-graph.cc

449

Empty line after this?

451

s/propper/proper/

462–463

Leave out the "I" and "Mathematica" parts -- explain the concepts involved, not the mechanics.

469–470

Instead of "subjectively provided a good result", say something like: "we used a 9-degree polynomial arbitrarily based on a fuzzy goodness of fit metric (using human judgment)".

483–484

Unnecessary now?

492–493

s/annother/another/

534–539

Do you need the equivalent of a "keep-going" flag here too (similar to what we have in llvm-xray account)?

[varno]

• Reply to comments

[dberris]

tools/llvm-xray/xray-graph.cc

534–539

Okay, now I think you need to report errors encountered that we're ignoring in case we do want to keep going. Accumulating the individual errors into a long list seems unnecessary to me.

540–546

This looks like it'd be better as:

if (auto E = GR.accountRecord(Record)) {
  ...
}

[varno]

• Respond to comments
• clang-format new

[varno]

• clang-format fix

[varno]

tools/llvm-xray/xray-graph.cc

540–546

This does not quite work here.

[dberris]

LGTM -- we can probably use some more user-facing documentation explaining a bit what this tool actually does (probably time to update docs/XRay.rst) but something to be done for later.

tools/llvm-xray/xray-graph.cc

539

In the future, comments like these really ought to expound more on what it's actually doing at a semantic level to help the reader of the code. A more informative comment as to "why" we're doing something might make more sense to the reader as that's not immediately obvious. Something like:

// Populate the graph with function call records that we
// encounter from the XRay trace. Any errors we encounter
// will be printed through standard error.

[dblaikie]

llvm/trunk/tools/llvm-xray/xray-graph.cc

212–213 ↗

(On Diff #85700)

Should probably be using llvm::errc - see the Support/errc.h - it defines portable constants we know work across all the compilers LLVM supports, etc.

401–403 ↗

(On Diff #85700)

Usually don't bother with {} on single line scopes.

586–592 ↗

(On Diff #85700)

Should accountRecord do this work/include this information in its Error (rather than callers doing so)?

llvm/trunk/tools/llvm-xray/xray-graph.h

135 ↗

(On Diff #85700)

Const value return type? I'm assuming this should be a const ref return type? (we should really have a clang-tidy for that if we don't already, maybe even a straight-up compiler warning)

tools/llvm-xray/xray-graph.h

 //
 //===----------------------------------------------------------------------===//
 
 #ifndef XRAY_GRAPH_H
 #define XRAY_GRAPH_H
 
 #include <unordered_map>
 #include <vector>
+#include <string>
 
 #include "func-id-helper.h"
 #include "xray-record.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/Support/raw_ostream.h"
 
 namespace llvm {
 namespace xray {
 
 /// A class encapsulating the logic related to analyzing XRay traces, producting
 /// Graphs from them and then exporting those graphs for review.
 class GraphRenderer {
 public:
-  /// An inner struct for common timing statistics information
+
+  /// An enum for enumerating the various statistics gathered on latencies
+  enum class StatType {NONE, COUNT, MIN, MED, PCT90, PCT99, MAX, SUM };
+
   struct TimeStat {
-    uint64_t Count;
-    double Min;
-    double Median;
-    double Pct90;
-    double Pct99;
-    double Max;
-    double Sum;
+    uint64_t Count = 0;
+    double Min = 0;
+    double Median = 0;
+    double Pct90 = 0;
+    double Pct99 = 0;
+    double Max = 0;
+    double Sum = 0;
+    std::string getAsString(StatType T) const;
+    double compare(StatType T,const TimeStat &Other) const;
   };
 
   /// An inner struct for storing edge attributes for our graph. Here the
   /// attributes are mainly function call statistics.
   ///
   /// FIXME: expand to contain more information eg call latencies.
   struct EdgeAttribute {
     TimeStat S;
private:
   /// The Graph stored in an edge-list like format, with the edges also having
   /// An attached set of attributes.
   DenseMap<int32_t, DenseMap<int32_t, EdgeAttribute>> Graph;
 
   /// Graph Vertex Attributes. These are presently stored seperate from the
   /// main graph.
   DenseMap<int32_t, VertexAttribute> VertexAttrs;
 
+  TimeStat GraphEdgeMax;
+  TimeStat GraphVertexMax;
+
   struct FunctionAttr {
     int32_t FuncId;
     uint64_t TSC;
   };
 
   /// Use a Map to store the Function stack for each thread whilst building the
   /// graph.
   ///
   /// FIXME: Perhaps we can Build this into LatencyAccountant? or vise versa?
   DenseMap<pid_t, SmallVector<FunctionAttr, 4>> PerThreadFunctionStack;
 
   /// Usefull object for getting human readable Symbol Names.
   FuncIdConversionHelper &FuncIdHelper;
   bool DeduceSiblingCalls = false;
   uint64_t CurrentMaxTSC = 0;
 
   /// A private function to help implement the statistic generation functions;
   template <typename U>
   void getStats(U begin, U end, GraphRenderer::TimeStat &S);
+  void updateMaxStats(const TimeStat &S, TimeStat &M);
 
   /// Calculates latency statistics for each edge and stores the data in the
   /// Graph
   void calculateEdgeStatistics();
 
   /// Calculates latency statistics for each vertex and stores the data in the
   /// Graph
   void calculateVertexStatistics();
public:
   ///
   /// This Function will return true on success, or false if records are not
   /// presented in per-thread call-tree DFS order. (That is for each thread the
   /// Records should be in order runtime on an ideal system.)
   ///
   /// FIXME: Make this more robust against small irregularities.
   bool accountRecord(const XRayRecord &Record);
 
-  /// An enum for enumerating the various statistics gathered on latencies
-  enum class StatType { COUNT, MIN, MED, PCT90, PCT99, MAX, SUM };
 
   /// Output the Embedded graph in DOT format on \p OS, labeling the edges by
   /// \p T
   void exportGraphAsDOT(raw_ostream &OS, const XRayFileHeader &H,
-                        StatType T = StatType::COUNT);
+                        StatType EdgeLabel = StatType::NONE,
+                        StatType EdgeColor = StatType::NONE,
+                        StatType VertexLabel = StatType::NONE,
+                        StatType VertexColor = StatType::NONE);
 };
 }
 }
 
 #endif // XRAY_GRAPH_H

tools/llvm-xray/xray-graph.cc

 // Generate a DOT file to represent the function call graph encountered in
 // the trace.
 //
 //===----------------------------------------------------------------------===//
 #include <algorithm>
 #include <cassert>
 #include <system_error>
 #include <utility>
+#include <cmath>
 
 #include "xray-extract.h"
 #include "xray-graph.h"
 #include "xray-log-reader.h"
 #include "xray-registry.h"
 #include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/FormatVariadic.h"
 
static cl::opt<bool> GraphDeduceSiblingCalls(
     "deduce-sibling-calls",
     cl::desc("Deduce sibling calls when unrolling function call stacks"),
     cl::sub(Graph), cl::init(false));
 static cl::alias
     GraphDeduceSiblingCalls2("d", cl::aliasopt(GraphDeduceSiblingCalls),
                              cl::desc("Alias for -deduce-sibling-calls"),
                              cl::sub(Graph));
 
-static cl::opt<GraphRenderer::StatType>
-    GraphEdgeLabel("edge-label",
-                   cl::desc("Output graphs with edges labeled with this field"),
-                   cl::value_desc("field"), cl::sub(Graph),
-                   cl::init(GraphRenderer::StatType::COUNT),
-                   cl::values(clEnumValN(GraphRenderer::StatType::COUNT,
-                                         "count", "function call counts"),
-                              clEnumValN(GraphRenderer::StatType::MIN, "min",
-                                         "minimum function durations"),
-                              clEnumValN(GraphRenderer::StatType::MED, "med",
-                                         "median function durations"),
-                              clEnumValN(GraphRenderer::StatType::PCT90, "90p",
-                                         "90th percentile durations"),
-                              clEnumValN(GraphRenderer::StatType::PCT99, "99p",
-                                         "99th percentile durations"),
-                              clEnumValN(GraphRenderer::StatType::MAX, "max",
-                                         "maximum function durations"),
-                              clEnumValN(GraphRenderer::StatType::SUM, "sum",
-                                         "sum of call durations")));
+static cl::opt<GraphRenderer::StatType> GraphEdgeLabel("edge-label",
+    cl::init(GraphRenderer::StatType::NONE),
+    cl::values(clEnumValN(GraphRenderer::StatType::NONE,
+                          "none", "Do not label Edges"),
+               clEnumValN(GraphRenderer::StatType::COUNT,
 static cl::alias GraphEdgeLabel2("e", cl::aliasopt(GraphEdgeLabel),
                                  cl::desc("Alias for -edge-label"),
                                  cl::sub(Graph));
 
+static cl::opt<GraphRenderer::StatType> GraphVertexLabel("vertex-label",
+    cl::desc("Output graphs with vertices labeled with this field"),
+    cl::value_desc("field"), cl::sub(Graph),
+    cl::init(GraphRenderer::StatType::NONE),
+    cl::values(clEnumValN(GraphRenderer::StatType::NONE,
+                          "none", "Do not label Edges"),
+               clEnumValN(GraphRenderer::StatType::COUNT,
+                          "count", "function call counts"),
+               clEnumValN(GraphRenderer::StatType::MIN, "min",
+                          "minimum function durations"),
+               clEnumValN(GraphRenderer::StatType::MED, "med",
+                          "median function durations"),
+               clEnumValN(GraphRenderer::StatType::PCT90, "90p",
+                          "90th percentile durations"),
+               clEnumValN(GraphRenderer::StatType::PCT99, "99p",
+                          "99th percentile durations"),
+               clEnumValN(GraphRenderer::StatType::MAX, "max",
+                          "maximum function durations"),
+               clEnumValN(GraphRenderer::StatType::SUM, "sum",
+                          "sum of call durations")));
+static cl::alias GraphVertexLabel2("v", cl::aliasopt(GraphEdgeLabel),
+                                 cl::desc("Alias for -edge-label"),
+                                 cl::sub(Graph));
+
+static cl::opt<GraphRenderer::StatType> GraphEdgeColorType("color-edges",
+    cl::desc("Output graphs with edge colors determined by this field"),
+    cl::value_desc("field"), cl::sub(Graph),
+    cl::init(GraphRenderer::StatType::NONE),
+    cl::values(clEnumValN(GraphRenderer::StatType::NONE,
+                          "none", "Do not label Edges"),
+               clEnumValN(GraphRenderer::StatType::COUNT,
+                          "count", "function call counts"),
+               clEnumValN(GraphRenderer::StatType::MIN, "min",
+                          "minimum function durations"),
+               clEnumValN(GraphRenderer::StatType::MED, "med",
+                          "median function durations"),
+               clEnumValN(GraphRenderer::StatType::PCT90, "90p",
+                          "90th percentile durations"),
+               clEnumValN(GraphRenderer::StatType::PCT99, "99p",
+                          "99th percentile durations"),
+               clEnumValN(GraphRenderer::StatType::MAX, "max",
+                          "maximum function durations"),
+               clEnumValN(GraphRenderer::StatType::SUM, "sum",
+                          "sum of call durations")));
+static cl::alias GraphEdgeColorType2("c", cl::aliasopt(GraphEdgeColorType),
+                                     cl::desc("Alias for -color-edges"),
+                                     cl::sub(Graph));
+
+static cl::opt<GraphRenderer::StatType> GraphVertexColorType("color-vertices",
+    cl::desc("Output graphs with vertex colors determined by this field"),
+    cl::value_desc("field"), cl::sub(Graph),
+    cl::init(GraphRenderer::StatType::NONE),
+    cl::values(clEnumValN(GraphRenderer::StatType::NONE,
+                          "none", "Do not label Edges"),
+               clEnumValN(GraphRenderer::StatType::COUNT,
+                          "count", "function call counts"),
+               clEnumValN(GraphRenderer::StatType::MIN, "min",
+                          "minimum function durations"),
+               clEnumValN(GraphRenderer::StatType::MED, "med",
+                          "median function durations"),
+               clEnumValN(GraphRenderer::StatType::PCT90, "90p",
+                          "90th percentile durations"),
+               clEnumValN(GraphRenderer::StatType::PCT99, "99p",
+                          "99th percentile durations"),
+               clEnumValN(GraphRenderer::StatType::MAX, "max",
+                          "maximum function durations"),
+               clEnumValN(GraphRenderer::StatType::SUM, "sum",
+                          "sum of call durations")));
+static cl::alias GraphVertexColorType2("b", cl::aliasopt(GraphEdgeLabel),
+                                 cl::desc("Alias for -edge-label"),
+                                 cl::sub(Graph));
 namespace {
 template <class T> T diff(T L, T R) { return std::max(L, R) - std::min(L, R); }
 
 void updateStat(GraphRenderer::TimeStat &S, int64_t lat) {
   S.Count++;
   if (S.Min > lat || S.Min == 0)
     S.Min = lat;
   if (S.Max < lat)
     S.Max = lat;
   S.Sum += lat;
 }
 }
 
 // Evaluates an XRay record and performs accounting on it, creating and
 // decorating a function call graph as it does so. It does this by maintaining
 // a call stack on a per-thread basis and adding edges and verticies to the
 // graph as they are seen for the first time.
 //
 // There is an immaginary root for functions at the top of their stack with
 // FuncId 0.
 //
 // FIXME: make more robust to errors and
 // Decorate Graph More Heavily.
 // FIXME: Refactor this and account subcommand to reduce code duplication.
 bool GraphRenderer::accountRecord(const XRayRecord &Record) {
   if (CurrentMaxTSC == 0)
     CurrentMaxTSC = Record.TSC;
 
void GraphRenderer::getStats(U begin, U end, GraphRenderer::TimeStat &S) {
   std::ptrdiff_t Pct90Off = (S.Count * 9) / 10;
   std::nth_element(begin, begin + Pct90Off, end);
   S.Pct90 = *(begin + Pct90Off);
   std::ptrdiff_t Pct99Off = (S.Count * 99) / 100;
   std::nth_element(begin, begin + Pct99Off, end);
   S.Pct99 = *(begin + Pct99Off);
 }
 
+void GraphRenderer::updateMaxStats(const GraphRenderer::TimeStat &S,
+                                       GraphRenderer::TimeStat &M){
+  M.Count  = std::max(M.Count,  S.Count);
+  M.Min    = std::max(M.Min,    S.Min);
+  M.Median = std::max(M.Median, S.Median);
+  M.Pct90  = std::max(M.Pct90,  S.Pct90);
+  M.Pct99  = std::max(M.Pct99,  S.Pct99);
+  M.Max    = std::max(M.Max,    S.Max);
+  M.Sum    = std::max(M.Sum,    S.Sum);
+}
+
 void GraphRenderer::calculateEdgeStatistics() {
   for (auto &V : Graph) {
     for (auto &E : V.second) {
       auto &A = E.second;
       getStats(A.Timings.begin(), A.Timings.end(), A.S);
+      updateMaxStats(A.S, GraphEdgeMax);
     }
   }
 }
 
 void GraphRenderer::calculateVertexStatistics() {
   DenseMap<int32_t, std::pair<uint64_t, SmallVector<EdgeAttribute *, 4>>>
       IncommingEdges;
   uint64_t MaxCount = 0;
void GraphRenderer::calculateVertexStatistics() {
   std::vector<uint64_t> TempTimings;
   TempTimings.reserve(MaxCount);
   for (auto &V : IncommingEdges) {
     for (auto &P : V.second.second) {
       TempTimings.insert(TempTimings.end(), P->Timings.begin(),
                          P->Timings.end());
     }
     getStats(TempTimings.begin(), TempTimings.end(), VertexAttrs[V.first].S);
+    updateMaxStats(VertexAttrs[V.first].S, GraphVertexMax);
     TempTimings.clear();
   }
-// Here is an alternative body of this function
-#if 0
-  DenseMap<int32_t, Smallvector<uint64_t, 5>> VertexData;
-  for (auto &V : Graph) {
-    for (auto &E : V.second){
-      auto &vect = VertexData[E.first];
-      vect.insert(vect.end(), E.second.Timings.begin(), E.second.timings.end());
-    }
-  }
-  for (auto &V: VertexData)
-    getStats(V.second.begin(), V.second.end(), VertexAttrs[V.first].S);
-#endif
 }
 
-void GraphRenderer::normaliseStatistics(double CycleFrequency) {
-  for (auto &V : Graph) {
-    for (auto &E : V.second) {
-      auto &S = E.second.S;
+namespace {
+// A Helper functio for Normalise Statistics which normalises a single
+// TimeStat element.
+void NormaliseTimeStat(GraphRenderer::TimeStat &S, double CycleFrequency){
       S.Min /= CycleFrequency;
       S.Median /= CycleFrequency;
       S.Max /= CycleFrequency;
       S.Sum /= CycleFrequency;
       S.Pct90 /= CycleFrequency;
       S.Pct99 /= CycleFrequency;
-    }
-  }
+
+// Normalises the statistics in the graph for a given TSC frequency.
+void GraphRenderer::normaliseStatistics(double CycleFrequency) {
+  for (auto &V : Graph) {
+    for (auto &E : V.second) {
+      auto &S = E.second.S;
+      NormaliseTimeStat(S,CycleFrequency);
+    }
+  }
   for (auto &V : VertexAttrs) {
     auto &S = V.second.S;
-    S.Min /= CycleFrequency;
-    S.Median /= CycleFrequency;
-    S.Max /= CycleFrequency;
-    S.Sum /= CycleFrequency;
-    S.Pct90 /= CycleFrequency;
-    S.Pct99 /= CycleFrequency;
+    NormaliseTimeStat(S,CycleFrequency);
+  }
+
+  NormaliseTimeStat(GraphEdgeMax, CycleFrequency);
+  NormaliseTimeStat(GraphVertexMax, CycleFrequency);
+}
+
+// Returns a string containing the value of statistic field T
+std::string GraphRenderer::TimeStat::getAsString(GraphRenderer::StatType T) const {
+  switch (T) {
+  case GraphRenderer::StatType::COUNT:
+    return std::to_string(Count);
+  case GraphRenderer::StatType::MIN:
+    return std::to_string(Min);
+  case GraphRenderer::StatType::MED:
+    return std::to_string(Median);
+  case GraphRenderer::StatType::PCT90:
+    return std::to_string(Pct90);
+  case GraphRenderer::StatType::PCT99:
+    return std::to_string(Pct99);
+  case GraphRenderer::StatType::MAX:
+    return std::to_string(Max);
+  case GraphRenderer::StatType::SUM:
+    return std::to_string(Sum);
+  case GraphRenderer::StatType::NONE:
+    std::string empty;
+    return empty;
   }
 }
 
-namespace {
-void outputEdgeInfo(const GraphRenderer::TimeStat &S, GraphRenderer::StatType T,
-                    raw_ostream &OS) {
+namespace{
+
+// Evaluates a polynomial p(x) = a_0 + a_1*x^1 + ... a_n*x^n at x_0 using
+// Horner's Method, and then returns the value as an 8-bit fixed point
+// representation
+// The Polynomial coefficients are parsed in such that begin contains a_n
+// and following it are a_(n-1) ... in order.
+template <typename U>
+uint8_t polyEval(const U &Begin, const U &End, double x_0){
+  double B = 0;
+  for(U i = Begin; i!= End; ++i){
+    B = *i + B*x_0;
+  }
+  double n = std::max(std::min(B,1.0),0.0);
+  return static_cast<uint8_t>(255*n + 0.5);
+}
+
+// Given a number between 0.0 and 1.0 returns a color from a gradient
+// defined between these two points, the gradient is represented as a
+// polynomial which is then evaluated at this point and scaled to an 24-bit
+// RGB value which is then returned as a string.
+std::string getColor(double point){
+  assert(point >= 0.0 && point <= 1);
+  const static double RedPoly[]   = {-38.4295, 239.239, -600.108, 790.544,
+                                     -591.26, 251.304, -58.0983, 6.62999,
+                                     -0.325899, 1.00173};
+  const static double GreenPoly[] = {-603.634, 2338.15, -3606.74, 2786.16,
+                                     -1085.19, 165.15, 11.2584, -6.11338,
+                                     -0.0091078, 0.965469};
+  const static double BluePoly[]  = {-325.686, 947.415, -699.079, -513.75,
+                                     1127.78, -732.617, 228.092, -33.8202,
+                                     0.732108, 0.913916};
+  using std::begin;
+  using std::end;
+  uint8_t r = polyEval(begin(RedPoly),end(RedPoly), point);
+  uint8_t g = polyEval(begin(GreenPoly),end(GreenPoly),point);
+  uint8_t b = polyEval(begin(BluePoly),end(BluePoly),point);
+
+  return llvm::formatv("#{0:X-2}{1:X-2}{2:x-2}", r, g, b);
+}
+}
+
+
+// Returns the quotient between the property T of this and annother TimeStat as
+// a double
+double GraphRenderer::TimeStat::compare(StatType T, const TimeStat &O) const{
+  double retval = 0;
   switch (T) {
   case GraphRenderer::StatType::COUNT:
-    OS << S.Count;
+    retval = static_cast<double>(Count)/static_cast<double>(O.Count);
     break;
   case GraphRenderer::StatType::MIN:
-    OS << S.Min;
+    retval = Min/O.Min;
     break;
   case GraphRenderer::StatType::MED:
-    OS << S.Median;
+    retval = Median/O.Median;
     break;
   case GraphRenderer::StatType::PCT90:
-    OS << S.Pct90;
+    retval = Pct90/O.Pct90;
     break;
   case GraphRenderer::StatType::PCT99:
-    OS << S.Pct99;
+    retval = Pct99/O.Pct99;
     break;
   case GraphRenderer::StatType::MAX:
-    OS << S.Max;
+    retval = Max/O.Max;
     break;
   case GraphRenderer::StatType::SUM:
-    OS << S.Sum;
+    retval = Sum/O.Sum;
+    break;
+  case GraphRenderer::StatType::NONE:
+    retval = 0.0;
     break;
   }
-}
+  return std::sqrt(retval);
 }
 
 // Does what the name suggests, it creates a DOT file from the Graph embedded in
 // the GraphRenderer object. It does this in the expected way by itterating
 // through all edges then vertices and then outputting them and their
 // annotations.
 //
 // FIXME: output more information, better presented.
 void GraphRenderer::exportGraphAsDOT(raw_ostream &OS, const XRayFileHeader &H,
-                                     StatType T) {
+                                     StatType ET, StatType EC, StatType VT, StatType VC) {
   calculateEdgeStatistics();
   calculateVertexStatistics();
   if (H.CycleFrequency)
     normaliseStatistics(H.CycleFrequency);
 
   OS << "digraph xray {\n";
 
+  if(VT != StatType::NONE)
+    OS << "node [shape=record];\n";
+
   for (const auto &V : Graph)
     for (const auto &E : V.second) {
+      const auto &S = E.second.S;
       OS << "F" << V.first << " -> "
-         << "F" << E.first << " [label=\"";
-      outputEdgeInfo(E.second.S, T, OS);
-      OS << "\"];\n";
+         << "F" << E.first << " [label=\""
+         << S.getAsString(ET) << "\"";
+      if (EC != StatType::NONE)
+         OS << " color=\"" << getColor(S.compare(EC,GraphEdgeMax)) << "\"";
+      OS << "];\n";
     }
 
-  for (const auto &V : VertexAttrs)
+  for (const auto &V : VertexAttrs){
+    const auto &VA = V.second;
     OS << "F" << V.first << " [label=\""
-       << (V.second.SymbolName.size() > 40
-               ? V.second.SymbolName.substr(0, 40) + "..."
-               : V.second.SymbolName)
-       << "\"];\n";
-
+       << (VT != StatType::NONE ? "{" : "")
+       << (VA.SymbolName.size() > 40
+               ? VA.SymbolName.substr(0, 40) + "..."
+               : VA.SymbolName);
+    if (VT != StatType::NONE)
+      OS << "|" << VA.S.getAsString(VT) << "}\"";
+    else
+      OS << "\"";
+    if (VC != StatType::NONE)
+      OS << " color=\"" << getColor(VA.S.compare(VC,GraphVertexMax)) << "\"";
+    OS << "];\n";
+  }
   OS << "}\n";
 }
 
 // Here we register and implement the llvm-xray graph subcommand.
 // The bulk of this code reads in the options, opens the required files, uses
 // those files to create a context for analysing the xray trace, then there is a
 // short loop which actually analyses the trace, generates the graph and then
 // outputs it as a DOT.
 //
 // FIXME: include additional filtering and annalysis passes to provide more
 // specific useful information.
 static CommandRegistration Unused(&Graph, []() -> Error {
   int Fd;
   auto EC = sys::fs::openFileForRead(GraphInput, Fd);
   if (EC)
     return make_error<StringError>(
         Twine("Cannot open file '") + GraphInput + "'", EC);
 
   Error Err = Error::success();
   xray::InstrumentationMapExtractor Extractor(GraphInstrMap, InstrMapFormat,
                                               Err);
   if (auto E = handleErrors(
           std::move(Err), [&](std::unique_ptr<StringError> E) -> Error {
             if (E->convertToErrorCode() == std::errc::no_such_file_or_directory)
               return Error::success();
             return Error(std::move(E));
           }))
     return E;
 
   raw_fd_ostream OS(GraphOutput, EC, sys::fs::OpenFlags::F_Text);
   if (EC)
     return make_error<StringError>(
         Twine("Cannot open file '") + GraphOutput + "' for writing.", EC);
 
   const auto &FunctionAddresses = Extractor.getFunctionAddresses();
   symbolize::LLVMSymbolizer::Options Opts(
       symbolize::FunctionNameKind::LinkageName, true, true, false, "");
   symbolize::LLVMSymbolizer Symbolizer(Opts);
   llvm::xray::FuncIdConversionHelper FuncIdHelper(GraphInstrMap, Symbolizer,
                                                   FunctionAddresses);
   xray::GraphRenderer GR(FuncIdHelper, GraphDeduceSiblingCalls);
   const llvm::xray::XRayFileHeader *Header = nullptr;
   LogReader Reader(GraphInput, Err, true, getSupportedLoader(GraphInputFormat));
   if (Err)
     return joinErrors(
         make_error<StringError>(
             Twine("Failed loading input file '") + GraphInput + "'",
             std::make_error_code(std::errc::protocol_error)),
         std::move(Err));
 
   Header = &Reader.getFileHeader();
   for (const auto &Record : Reader) {
     // Generate graph, FIXME: better error recovery.
     if (!GR.accountRecord(Record)) {
       return make_error<StringError>(
           Twine("Failed accounting function calls in file '") + GraphInput +
               "'.",
           std::make_error_code(std::errc::bad_message));
     }
   }
 
-  GR.exportGraphAsDOT(OS, *Header, GraphEdgeLabel);
+  GR.exportGraphAsDOT(OS, *Header,
+                      GraphEdgeLabel, GraphEdgeColorType,
+                      GraphVertexLabel, GraphVertexColorType);
   return Error::success();
 });