Sync machine code even if codegen is skipped #512
Conversation
| } | ||
|
|
||
| #define CHECK_ERRORS(response) \ | ||
| #define CHECK_ERRORS(response, bucketName, keyName) \ |
There was a problem hiding this comment.
I also update this macro as otherwise the error messages would be of very little help.
There was a problem hiding this comment.
I agree, these errors are usually pretty hard to parse. I'm not sure the no key and no bucket messages are paricularly clear in the current form though. Instead we could have a conditional and log the error without a key or bucket if there isn't one, e.g.
#define CHECK_ERRORS(response, bucketName, keyName)
{
if (!response.IsSuccess()) {
const auto& err = response.GetError();
if(bucketName.empty()) {
SPDLOG_ERROR("General S3 error");
else if(keyName.empty()) {
SPDLOG_ERROR("S3 error for bucket: {}", bucketName);
} else {
SPDLOG_ERROR("S3 error with bucket/key: {}/{}", bucketName, keyName);
}
...
}
}
...
CHECK_ERRORS(response, "", "");
CHECK_ERRORS(response, bucketName, "");
CHECK_ERRORS(response, bucketName, keyName);
42a86a9 to
664cf8e
Compare
1606f80 to
1d2a0e5
Compare
src/codegen/MachineCodeGenerator.cpp
Outdated
| if (conf.wasmVm == "wamr") { | ||
| (void)loader.loadFunctionWamrAotHash(msg); | ||
| } else { | ||
| (void)loader.loadFunctionObjectFile(msg); |
There was a problem hiding this comment.
What's this void cast for? If it's to deal with warnings, we have an UNUSED macro.
There was a problem hiding this comment.
Good point. That being said, the UNUSED macro gives a compiler warning itself because it clashes with an UNUSED definition in pistache 🤣
I suggest we eventually change it to FAABRIC_UNUSED.
There was a problem hiding this comment.
Haha yes, clearly great minds think alike (or both read the same SO post...)
| } | ||
|
|
||
| #define CHECK_ERRORS(response) \ | ||
| #define CHECK_ERRORS(response, bucketName, keyName) \ |
There was a problem hiding this comment.
I agree, these errors are usually pretty hard to parse. I'm not sure the no key and no bucket messages are paricularly clear in the current form though. Instead we could have a conditional and log the error without a key or bucket if there isn't one, e.g.
#define CHECK_ERRORS(response, bucketName, keyName)
{
if (!response.IsSuccess()) {
const auto& err = response.GetError();
if(bucketName.empty()) {
SPDLOG_ERROR("General S3 error");
else if(keyName.empty()) {
SPDLOG_ERROR("S3 error for bucket: {}", bucketName);
} else {
SPDLOG_ERROR("S3 error with bucket/key: {}/{}", bucketName, keyName);
}
...
}
}
...
CHECK_ERRORS(response, "", "");
CHECK_ERRORS(response, bucketName, "");
CHECK_ERRORS(response, bucketName, keyName);
7931839 to
2d6dfe6
Compare
Shillaker
left a comment
There was a problem hiding this comment.
Nice, just the one UNUSED.
src/codegen/MachineCodeGenerator.cpp
Outdated
| if ((!oldHash.empty()) && newHash == oldHash) { | ||
| // Even if we skip the code generation step, we want to sync the latest | ||
| // shared object object file | ||
| (void)loader.loadSharedObjectObjectFile(inputPath); |
2d6dfe6 to
473f67d
Compare
No description provided.