[PATCH] D24376: [XRay] Implement `llvm-xray convert` -- trace file conversion

[David Blaikie via Phabricator via llvm-commits]

dblaikie accepted this revision.
dblaikie added a comment.
This revision is now accepted and ready to land.

> Good point. So I think "conversion" is a bit more loaded than just "dump", where "dump" really just makes the representation more human-readable (for debugging, or just turning machine-readable form to human-readable form).
> 
> Will renaming this from 'convert' to 'dump' make that better?

I don't think that'd be accurate - this is doing format conversions between semantic preserving representations intended for machine consumption. Our dumping tools are generally intended only for human consumption (or really hacky machine consumption in scripts for quick work sometimes (eg: some scripts I'm writing using objdump/dwarfdump for doing some object size analysis)).



================
Comment at: tools/llvm-xray/xray-extract.cc:220
+    FunctionIds[Y.Function] = Y.FuncId;
+    Sleds.emplace_back(
+        SledEntry{Y.Address, Y.Function, Y.Kind, Y.AlwaysInstrument});
----------------
push_back? (since you're specifying the type anyway, etc)


================
Comment at: tools/llvm-xray/xray-log-reader.cc:165
+                 });
+  Records = std::move(Tmp);
+  return Error::success();
----------------
I'd skip the 'Tmp' temporary, and put the result directly into 'Records' (because it seems simpler - but arguably could have slightly improved performance if 'Records' already has a large capacity that can be reused)


https://reviews.llvm.org/D24376