[PATCH] D34863: [XRay][tools] Function call stack based analysis tooling for XRay traces

Dean Michael Berris via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 6 19:05:04 PDT 2017


dberris accepted this revision.
dberris added a comment.

LGTM -- just a couple of readability suggestions. Otherwise, good to land. :)



================
Comment at: tools/llvm-xray/xray-stacks.cc:256-258
+  for (auto MapPairIter : LeftCalleesByFuncId) {
+    Node->Callees.push_back(MapPairIter.second);
+  }
----------------
Is it possible to just do something like:

```
auto &RightCallees = MapPairIter.second;
Node->Callees.insert(Node->Callees.end(), RightCallees.begin(), RightCallees.end());
```

instead?


================
Comment at: tools/llvm-xray/xray-stacks.cc:261-263
+  for (auto duration : Left.TerminalDurations) {
+    Node->TerminalDurations.push_back(duration);
+  }
----------------
This one seems simpler as:

```
Node->TerminalDurations.insert(
  Node->TerminalDurations.end(),
  Left.TerminalDurations.begin(), Left.TerminalDurations.end());
```

Or, if there's an `append` that takes a range, it might be more efficient/simpler than growing the containers one element at a time.


https://reviews.llvm.org/D34863





More information about the llvm-commits mailing list