Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ctx_prof] Move the "from json" logic more centrally to reuse it from test. #106129

Conversation

mtrofin
Copy link
Member

@mtrofin mtrofin commented Aug 26, 2024

Making the synthesis of a contextual profile file from a JSON descriptor more reusable, for unittest authoring purposes.

The functionality round-trips through the binary format - no reason, currently, to support other ways of loading contextual profiles.

Issue #89287

Copy link
Member Author

mtrofin commented Aug 26, 2024

@mtrofin mtrofin marked this pull request as ready for review August 26, 2024 20:25
@llvmbot llvmbot added the PGO Profile Guided Optimizations label Aug 26, 2024
@llvmbot
Copy link
Member

llvmbot commented Aug 26, 2024

@llvm/pr-subscribers-pgo

Author: Mircea Trofin (mtrofin)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/106129.diff

3 Files Affected:

  • (modified) llvm/include/llvm/ProfileData/PGOCtxProfWriter.h (+1)
  • (modified) llvm/lib/ProfileData/PGOCtxProfWriter.cpp (+82)
  • (modified) llvm/tools/llvm-ctxprof-util/llvm-ctxprof-util.cpp (+7-75)
diff --git a/llvm/include/llvm/ProfileData/PGOCtxProfWriter.h b/llvm/include/llvm/ProfileData/PGOCtxProfWriter.h
index db9a0fd77f8351..b370fdd9ba5a1c 100644
--- a/llvm/include/llvm/ProfileData/PGOCtxProfWriter.h
+++ b/llvm/include/llvm/ProfileData/PGOCtxProfWriter.h
@@ -81,5 +81,6 @@ class PGOCtxProfileWriter final {
   static constexpr StringRef ContainerMagic = "CTXP";
 };
 
+Error createCtxProfFromJSON(StringRef Profile, raw_ostream &Out);
 } // namespace llvm
 #endif
diff --git a/llvm/lib/ProfileData/PGOCtxProfWriter.cpp b/llvm/lib/ProfileData/PGOCtxProfWriter.cpp
index 74cd8763cc769d..99b5b2b3d05811 100644
--- a/llvm/lib/ProfileData/PGOCtxProfWriter.cpp
+++ b/llvm/lib/ProfileData/PGOCtxProfWriter.cpp
@@ -12,6 +12,8 @@
 
 #include "llvm/ProfileData/PGOCtxProfWriter.h"
 #include "llvm/Bitstream/BitCodeEnums.h"
+#include "llvm/ProfileData/CtxInstrContextNode.h"
+#include "llvm/Support/JSON.h"
 
 using namespace llvm;
 using namespace llvm::ctx_profile;
@@ -81,3 +83,83 @@ void PGOCtxProfileWriter::writeImpl(std::optional<uint32_t> CallerIndex,
 void PGOCtxProfileWriter::write(const ContextNode &RootNode) {
   writeImpl(std::nullopt, RootNode);
 }
+
+namespace {
+// A structural representation of the JSON input.
+struct DeserializableCtx {
+  ctx_profile::GUID Guid = 0;
+  std::vector<uint64_t> Counters;
+  std::vector<std::vector<DeserializableCtx>> Callsites;
+};
+
+ctx_profile::ContextNode *
+createNode(std::vector<std::unique_ptr<char[]>> &Nodes,
+           const std::vector<DeserializableCtx> &DCList);
+
+// Convert a DeserializableCtx into a ContextNode, potentially linking it to
+// its sibling (e.g. callee at same callsite) "Next".
+ctx_profile::ContextNode *
+createNode(std::vector<std::unique_ptr<char[]>> &Nodes,
+           const DeserializableCtx &DC,
+           ctx_profile::ContextNode *Next = nullptr) {
+  auto AllocSize = ctx_profile::ContextNode::getAllocSize(DC.Counters.size(),
+                                                          DC.Callsites.size());
+  auto *Mem = Nodes.emplace_back(std::make_unique<char[]>(AllocSize)).get();
+  std::memset(Mem, 0, AllocSize);
+  auto *Ret = new (Mem) ctx_profile::ContextNode(DC.Guid, DC.Counters.size(),
+                                                 DC.Callsites.size(), Next);
+  std::memcpy(Ret->counters(), DC.Counters.data(),
+              sizeof(uint64_t) * DC.Counters.size());
+  for (const auto &[I, DCList] : llvm::enumerate(DC.Callsites))
+    Ret->subContexts()[I] = createNode(Nodes, DCList);
+  return Ret;
+}
+
+// Convert a list of DeserializableCtx into a linked list of ContextNodes.
+ctx_profile::ContextNode *
+createNode(std::vector<std::unique_ptr<char[]>> &Nodes,
+           const std::vector<DeserializableCtx> &DCList) {
+  ctx_profile::ContextNode *List = nullptr;
+  for (const auto &DC : DCList)
+    List = createNode(Nodes, DC, List);
+  return List;
+}
+} // namespace
+
+namespace llvm {
+namespace json {
+bool fromJSON(const Value &E, DeserializableCtx &R, Path P) {
+  json::ObjectMapper Mapper(E, P);
+  return Mapper && Mapper.map("Guid", R.Guid) &&
+         Mapper.map("Counters", R.Counters) &&
+         Mapper.mapOptional("Callsites", R.Callsites);
+}
+} // namespace json
+} // namespace llvm
+
+Error llvm::createCtxProfFromJSON(StringRef Profile, raw_ostream &Out) {
+  auto P = json::parse(Profile);
+  if (!P)
+    return P.takeError();
+
+  json::Path::Root R("");
+  std::vector<DeserializableCtx> DCList;
+  if (!fromJSON(*P, DCList, R))
+    return R.getError();
+  // Nodes provides memory backing for the ContextualNodes.
+  std::vector<std::unique_ptr<char[]>> Nodes;
+  std::error_code EC;
+  if (EC)
+    return createStringError(EC, "failed to open output");
+  PGOCtxProfileWriter Writer(Out);
+  for (const auto &DC : DCList) {
+    auto *TopList = createNode(Nodes, DC);
+    if (!TopList)
+      return createStringError(
+          "Unexpected error converting internal structure to ctx profile");
+    Writer.write(*TopList);
+  }
+  if (EC)
+    return createStringError(EC, "failed to write output");
+  return Error::success();
+}
\ No newline at end of file
diff --git a/llvm/tools/llvm-ctxprof-util/llvm-ctxprof-util.cpp b/llvm/tools/llvm-ctxprof-util/llvm-ctxprof-util.cpp
index 3bb7681e33a871..0fad4ee4360ddf 100644
--- a/llvm/tools/llvm-ctxprof-util/llvm-ctxprof-util.cpp
+++ b/llvm/tools/llvm-ctxprof-util/llvm-ctxprof-util.cpp
@@ -46,90 +46,22 @@ static cl::opt<std::string> OutputFilename("output", cl::value_desc("output"),
                                            cl::desc("Output file"),
                                            cl::sub(FromJSON));
 
-namespace {
-// A structural representation of the JSON input.
-struct DeserializableCtx {
-  GlobalValue::GUID Guid = 0;
-  std::vector<uint64_t> Counters;
-  std::vector<std::vector<DeserializableCtx>> Callsites;
-};
-
-ctx_profile::ContextNode *
-createNode(std::vector<std::unique_ptr<char[]>> &Nodes,
-           const std::vector<DeserializableCtx> &DCList);
-
-// Convert a DeserializableCtx into a ContextNode, potentially linking it to
-// its sibling (e.g. callee at same callsite) "Next".
-ctx_profile::ContextNode *
-createNode(std::vector<std::unique_ptr<char[]>> &Nodes,
-           const DeserializableCtx &DC,
-           ctx_profile::ContextNode *Next = nullptr) {
-  auto AllocSize = ctx_profile::ContextNode::getAllocSize(DC.Counters.size(),
-                                                          DC.Callsites.size());
-  auto *Mem = Nodes.emplace_back(std::make_unique<char[]>(AllocSize)).get();
-  std::memset(Mem, 0, AllocSize);
-  auto *Ret = new (Mem) ctx_profile::ContextNode(DC.Guid, DC.Counters.size(),
-                                                 DC.Callsites.size(), Next);
-  std::memcpy(Ret->counters(), DC.Counters.data(),
-              sizeof(uint64_t) * DC.Counters.size());
-  for (const auto &[I, DCList] : llvm::enumerate(DC.Callsites))
-    Ret->subContexts()[I] = createNode(Nodes, DCList);
-  return Ret;
-}
-
-// Convert a list of DeserializableCtx into a linked list of ContextNodes.
-ctx_profile::ContextNode *
-createNode(std::vector<std::unique_ptr<char[]>> &Nodes,
-           const std::vector<DeserializableCtx> &DCList) {
-  ctx_profile::ContextNode *List = nullptr;
-  for (const auto &DC : DCList)
-    List = createNode(Nodes, DC, List);
-  return List;
-}
-} // namespace
-
-namespace llvm {
-namespace json {
-// Hook into the JSON deserialization.
-bool fromJSON(const Value &E, DeserializableCtx &R, Path P) {
-  json::ObjectMapper Mapper(E, P);
-  return Mapper && Mapper.map("Guid", R.Guid) &&
-         Mapper.map("Counters", R.Counters) &&
-         Mapper.mapOptional("Callsites", R.Callsites);
-}
-} // namespace json
-} // namespace llvm
-
 // Save the bitstream profile from the JSON representation.
 Error convertFromJSON() {
   auto BufOrError = MemoryBuffer::getFileOrSTDIN(InputFilename);
   if (!BufOrError)
     return createFileError(InputFilename, BufOrError.getError());
-  auto P = json::parse(BufOrError.get()->getBuffer());
-  if (!P)
-    return P.takeError();
 
-  std::vector<DeserializableCtx> DCList;
-  json::Path::Root R("");
-  if (!fromJSON(*P, DCList, R))
-    return R.getError();
-  // Nodes provides memory backing for the ContextualNodes.
-  std::vector<std::unique_ptr<char[]>> Nodes;
   std::error_code EC;
-  raw_fd_stream Out(OutputFilename, EC);
+  // Using a fd_ostream instead of a fd_stream. The latter would be more
+  // efficient as the bitstream writer supports incremental flush to it, but the
+  // json scenario is for test, and file size scalability doesn't really concern
+  // us.
+  raw_fd_ostream Out(OutputFilename, EC);
   if (EC)
     return createStringError(EC, "failed to open output");
-  PGOCtxProfileWriter Writer(Out);
-  for (const auto &DC : DCList) {
-    auto *TopList = createNode(Nodes, DC);
-    if (!TopList)
-      return createStringError(
-          "Unexpected error converting internal structure to ctx profile");
-    Writer.write(*TopList);
-  }
-  if (EC)
-    return createStringError(EC, "failed to write output");
-  return Error::success();
+
+  return llvm::createCtxProfFromJSON(BufOrError.get()->getBuffer(), Out);
 }
 
 int main(int argc, const char **argv) {

@mtrofin mtrofin requested a review from snehasish August 26, 2024 20:26
@mtrofin mtrofin force-pushed the users/mtrofin/08-26-_ctx_prof_move_the_from_json_logic_more_centrally_to_reuse_it_from_test branch from 9bfbc45 to 8f5ee9a Compare August 27, 2024 21:11
Copy link
Member Author

mtrofin commented Aug 27, 2024

Merge activity

  • Aug 27, 6:37 PM EDT: @mtrofin started a stack merge that includes this pull request via Graphite.
  • Aug 27, 6:40 PM EDT: Graphite rebased this pull request as part of a merge.
  • Aug 27, 6:43 PM EDT: @mtrofin merged this pull request with Graphite.

@mtrofin mtrofin force-pushed the users/mtrofin/08-26-_ctx_prof_move_the_from_json_logic_more_centrally_to_reuse_it_from_test branch from 8f5ee9a to 7b6b6ed Compare August 27, 2024 22:40
@mtrofin mtrofin merged commit 1022323 into main Aug 27, 2024
5 of 7 checks passed
@mtrofin mtrofin deleted the users/mtrofin/08-26-_ctx_prof_move_the_from_json_logic_more_centrally_to_reuse_it_from_test branch August 27, 2024 22:43
searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Aug 28, 2024
reverts: Depend upon older [ctx_prof] reverts
73c3b73 [ctx_prof] Add support for ICP (llvm#105469)
1022323 [ctx_prof] Move the "from json" logic more centrally to reuse it from test. (llvm#106129)

Change-Id: I7579b15b8df14981afa399ca966aab0741e8637e
5c4lar pushed a commit to 5c4lar/llvm-project that referenced this pull request Aug 29, 2024
… test. (llvm#106129)

Making the synthesis of a contextual profile file from a JSON descriptor more reusable, for unittest authoring purposes.

The functionality round-trips through the binary format - no reason, currently, to support other ways of loading contextual profiles.
searlmc1 pushed a commit to ROCm/llvm-project that referenced this pull request Sep 7, 2024
… test. (llvm#106129)

Making the synthesis of a contextual profile file from a JSON descriptor more reusable, for unittest authoring purposes.

The functionality round-trips through the binary format - no reason, currently, to support other ways of loading contextual profiles.

Change-Id: Idf6084fc12dd254e72c9f26987e50a87b4376f09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PGO Profile Guided Optimizations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants