-
Notifications
You must be signed in to change notification settings - Fork 16
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
Adding arbitrary information to execution graph #166
Changes from all commits
aace59d
7624ff9
7ba6cfe
5755bf9
a46a5e9
2a66807
46ab804
1b64990
02661a9
0103cbc
534ae9c
163ea28
d6c3b1a
11ec3fc
80e5521
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
#pragma once | ||
|
||
#include <faabric/proto/faabric.pb.h> | ||
|
||
#include <functional> | ||
#include <list> | ||
#include <map> | ||
|
||
namespace faabric::util::exec_graph { | ||
void addDetail(faabric::Message& msg, | ||
const std::string& key, | ||
const std::string& value); | ||
|
||
void incrementCounter(faabric::Message& msg, | ||
const std::string& key, | ||
const int valueToIncrement = 1); | ||
|
||
static inline std::string const mpiMsgCountPrefix = "mpi-msgcount-torank-"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This constant is MPI-specific and only used by MPI stuff, therefore should live in an MPI header (and I would also use a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know this is not how we define constants elsewhere, and I thought about the Orthogonally, the constant is MPI-specific but also exec-graph specific, i.e. it is only used to record this exec graph details, and is not needed in MPI headers. Thus why I placed it here, I can see us having some of this "prefix" strings together here; happy to move elsewhere though. |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,7 @@ faabric_lib(util | |
crash.cpp | ||
delta.cpp | ||
environment.cpp | ||
exec_graph.cpp | ||
files.cpp | ||
func.cpp | ||
gids.cpp | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
#include <faabric/util/exec_graph.h> | ||
#include <faabric/util/logging.h> | ||
#include <faabric/util/testing.h> | ||
|
||
namespace faabric::util::exec_graph { | ||
void addDetail(faabric::Message& msg, | ||
const std::string& key, | ||
const std::string& value) | ||
{ | ||
if (!msg.recordexecgraph()) { | ||
return; | ||
} | ||
|
||
auto& stringMap = *msg.mutable_execgraphdetails(); | ||
|
||
stringMap[key] = value; | ||
} | ||
|
||
void incrementCounter(faabric::Message& msg, | ||
const std::string& key, | ||
const int valueToIncrement) | ||
{ | ||
if (!msg.recordexecgraph()) { | ||
return; | ||
} | ||
|
||
auto& stringMap = *msg.mutable_intexecgraphdetails(); | ||
|
||
stringMap[key] += valueToIncrement; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -8,6 +8,8 @@ | |
|
||
#include <cppcodec/base64_rfc4648.hpp> | ||
|
||
#include <sstream> | ||
|
||
using namespace rapidjson; | ||
|
||
namespace faabric::util { | ||
|
@@ -183,6 +185,45 @@ std::string messageToJson(const faabric::Message& msg) | |
a); | ||
} | ||
|
||
if (msg.recordexecgraph()) { | ||
d.AddMember("record_exec_graph", msg.recordexecgraph(), a); | ||
|
||
if (msg.execgraphdetails_size() > 0) { | ||
std::stringstream ss; | ||
const auto& map = msg.execgraphdetails(); | ||
auto it = map.begin(); | ||
while (it != map.end()) { | ||
ss << fmt::format("{}:{}", it->first, it->second); | ||
++it; | ||
if (it != map.end()) { | ||
ss << ","; | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Won't this result in a leading Could instead do this with a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
|
||
std::string out = ss.str(); | ||
d.AddMember( | ||
"exec_graph_detail", Value(out.c_str(), out.size()).Move(), a); | ||
} | ||
|
||
if (msg.intexecgraphdetails_size() > 0) { | ||
std::stringstream ss; | ||
const auto& map = msg.intexecgraphdetails(); | ||
auto it = map.begin(); | ||
while (it != map.end()) { | ||
ss << fmt::format("{}:{}", it->first, it->second); | ||
++it; | ||
if (it != map.end()) { | ||
ss << ","; | ||
} | ||
} | ||
|
||
std::string out = ss.str(); | ||
d.AddMember("int_exec_graph_detail", | ||
Value(out.c_str(), out.size()).Move(), | ||
a); | ||
} | ||
} | ||
|
||
StringBuffer sb; | ||
Writer<StringBuffer> writer(sb); | ||
d.Accept(writer); | ||
|
@@ -266,6 +307,54 @@ std::string getStringFromJson(Document& doc, | |
return std::string(valuePtr, valuePtr + it->value.GetStringLength()); | ||
} | ||
|
||
std::map<std::string, std::string> getStringStringMapFromJson( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I actually completely overlooked the string to json conversion, and updating the |
||
Document& doc, | ||
const std::string& key) | ||
{ | ||
std::map<std::string, std::string> map; | ||
|
||
Value::MemberIterator it = doc.FindMember(key.c_str()); | ||
if (it == doc.MemberEnd()) { | ||
return map; | ||
} | ||
|
||
const char* valuePtr = it->value.GetString(); | ||
std::stringstream ss( | ||
std::string(valuePtr, valuePtr + it->value.GetStringLength())); | ||
std::string keyVal; | ||
while (std::getline(ss, keyVal, ',')) { | ||
auto pos = keyVal.find(":"); | ||
std::string key = keyVal.substr(0, pos); | ||
map[key] = keyVal.erase(0, pos + sizeof(char)); | ||
} | ||
|
||
return map; | ||
} | ||
|
||
std::map<std::string, int> getStringIntMapFromJson(Document& doc, | ||
const std::string& key) | ||
{ | ||
std::map<std::string, int> map; | ||
|
||
Value::MemberIterator it = doc.FindMember(key.c_str()); | ||
if (it == doc.MemberEnd()) { | ||
return map; | ||
} | ||
|
||
const char* valuePtr = it->value.GetString(); | ||
std::stringstream ss( | ||
std::string(valuePtr, valuePtr + it->value.GetStringLength())); | ||
std::string keyVal; | ||
while (std::getline(ss, keyVal, ',')) { | ||
auto pos = keyVal.find(":"); | ||
std::string key = keyVal.substr(0, pos); | ||
int val = std::stoi(keyVal.erase(0, pos + sizeof(char))); | ||
map[key] = val; | ||
} | ||
|
||
return map; | ||
} | ||
|
||
faabric::Message jsonToMessage(const std::string& jsonIn) | ||
{ | ||
PROF_START(jsonDecode) | ||
|
@@ -324,6 +413,28 @@ faabric::Message jsonToMessage(const std::string& jsonIn) | |
msg.set_sgxpolicy(getStringFromJson(d, "sgxpolicy", "")); | ||
msg.set_sgxresult(getStringFromJson(d, "sgxresult", "")); | ||
|
||
msg.set_recordexecgraph(getBoolFromJson(d, "record_exec_graph", false)); | ||
|
||
// By default, clear the map | ||
msg.clear_execgraphdetails(); | ||
// Fill keypairs if found | ||
auto& msgStrMap = *msg.mutable_execgraphdetails(); | ||
std::map<std::string, std::string> strMap = | ||
getStringStringMapFromJson(d, "exec_graph_detail"); | ||
for (auto& it : strMap) { | ||
msgStrMap[it.first] = it.second; | ||
} | ||
|
||
// By default, clear the map | ||
msg.clear_intexecgraphdetails(); | ||
// Fill keypairs if found | ||
auto& msgIntMap = *msg.mutable_intexecgraphdetails(); | ||
std::map<std::string, int> intMap = | ||
getStringIntMapFromJson(d, "int_exec_graph_detail"); | ||
for (auto& it : intMap) { | ||
msgIntMap[it.first] = it.second; | ||
} | ||
|
||
PROF_END(jsonDecode) | ||
|
||
return msg; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Need to de-constify this so that we can actually edit the message when tracing.