[PATCH] D56383: [XRay][tools] Use symbols instead of function id

Kristina Brooks via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 7 21:09:55 PST 2019


kristina added a reviewer: kristina.
kristina added a comment.

Added some inline comments.



================
Comment at: llvm/tools/llvm-xray/xray-graph-diff.cpp:349
   case GraphDiffRenderer::StatType::NONE:
-    return formatv(R"({0})", truncateString(VertexId, TrunLen).str());
+    return formatv(
+        R"({0})",
----------------
Drop the `formatv` and the raw literal. It's in the old code sure, but `formatv` implementation in LLVM is rather expensive for two braces and the raw literal here makes zero sense. Use a `Twine` instead. Same goes for a a few below.


================
Comment at: llvm/tools/llvm-xray/xray-graph-diff.cpp:358
+          truncateString(anySymbol(VertexAttr.CorrVertexPtr, VertexId), TrunLen)
+              .str());
 
----------------
`truncateString` returns a `Twine`, you're not storing it anywhere so it's still on the stack at that point, what's the point of the explicit conversion here? `.str()`. A lot of things including `formatv` can accept a `Twine` as an argument.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56383/new/

https://reviews.llvm.org/D56383





More information about the llvm-commits mailing list