[PATCH] D27243: Initial work on the XRay Graph tool.

[Alexis Shaw via Phabricator via llvm-commits]

varno marked 2 inline comments as done.
varno added inline comments.


================
Comment at: tools/llvm-xray/xray-graph.h:115-124
+  /// Calculates latency statistics for each edge and stores the data in the
+  /// Graph
+  void calculateEdgeStatistics(void);
+
+  ///Calculates latency statistics for each vertex and stores the data in the
+  /// Graph
+  void calculateVertexStatistics(void);
----------------
dberris wrote:
> varno wrote:
> > dberris wrote:
> > > varno wrote:
> > > > dberris wrote:
> > > > > Can this not happen incrementally, as we're adding the records, or on demand when we export? i.e. why do they need to be public functions?
> > > > Not really, all the records need to be added before we calculate the statistics. And I was wanting the statistics calculated to do graph transformations later (before output).
> > > Sure, but why aren't these just implementation details of the exportGraphAsDOT(...) function?
> > I suppose that right now they could be, but I have plans that require them to be run before exportGraphAsDOT
> I suspect we can break these out later if we really need to. For now, they're a distraction and from an API design perspective, really brittle -- if someone wants to use this class they have to know to call these functions before exporting. If this doesn't happen while we're accounting the records, then this means the state of the graph is not easily determined.
> 
> If for example an external algorithm requires the graph, then we ought to be able to access the graph itself -- and maybe the accounting ought to happen externally instead, as an algorithm that passes through the graph we've built or as an extension to the graph traversal algorithm being employed, exposed as a method to this class. 
Ok, I'll break them out and we can add them in later if needed


https://reviews.llvm.org/D27243