[PATCH] D135912: [PseudoProbe] Replace relocation with offset for entry probe.

[Rafael Auler via Phabricator via llvm-commits]

rafauler added a comment.

Not an expert, just some hints in case you like the suggestions.



================
Comment at: llvm/include/llvm/MC/MCPseudoProbe.h:293
+  // A collection of MCPseudoProbe for each binary function. The MCPseudoProbes
+  // are grouped by GUIDs due to inlining that can bring probes from different functions into one function.
   MCProbeDivisionMap MCProbeDivisions;
----------------
Is this over 80col?


================
Comment at: llvm/lib/MC/MCPseudoProbe.cpp:149
+  // top-level functions if needed.
+  bool NeedSantinel = false;
+  if (Parent->isRoot()) {
----------------
typo Santinel->Sentinel


================
Comment at: llvm/lib/MC/MCPseudoProbe.cpp:206-220
+      std::map<InlineSite, MCPseudoProbeInlineTree *> Inlinees;
+      for (auto Child = Root.getChildren().begin();
+           Child != Root.getChildren().end(); ++Child)
+        Inlinees[Child->first] = Child->second.get();
+
+      for (const auto &Inlinee : Inlinees) {
+        // Emit the group guarded by a sentinel probe.
----------------
std::map is an expensive data structure. For usage patterns that are strictly  "insert then query", considering using a vector + std::sort.

Reference https://llvm.org/docs/ProgrammersManual.html#dss-sortedvectormap


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D135912