Reduce log overhead when logging disabled #711
Conversation
|
|
||
| template <typename... Args> | ||
| inline static void log(const std::string& tag, const std::string& formatString, Args&&... args) { | ||
| std::string formattedString = string_format(formatString, std::forward<Args>(args)...); | ||
| log(tag, formattedString); | ||
| #ifdef DEBUG |
There was a problem hiding this comment.
this is redundant w/ the directive in iOS. It might be more correct to just guard iOS which has no concept of log levels, and leave Android based on log level. Or you could choose to have the directive here to have more similar behavior between iOS and Android
|
|
||
| public: | ||
| static void log(const std::string& tag, const std::string& message); | ||
| static void log(const std::string& tag, const std::string& formatString, Args... args); |
There was a problem hiding this comment.
This is not a template, you cannot use Args... here. This is why the C++ build is now failing.
There was a problem hiding this comment.
yes, I understand, but I ended up not using this project for my work, so I don't have time to fully flesh out this patch. I was intending to simply illustrate some ways of avoiding the logging overhead and/or avoiding NSLog in release builds (which is discouraged)
| @@ -8,9 +8,11 @@ | |||
| #import "MmkvLogger.h" | |||
| #import <Foundation/Foundation.h> | |||
|
|
|||
| void MmkvLogger::log(const std::string& tag, const std::string& message) { | |||
| void MmkvLogger::log(const std::string& tag, const std::string& formatString, Args... args) { | |||
There was a problem hiding this comment.
same here, not a template.
we can't even template this because our implementation is in two differnt spots (ios and android). so we might not be able to support Args....
| @@ -8,9 +8,9 @@ | |||
| #include "MmkvLogger.h" | |||
| #include <android/log.h> | |||
|
|
|||
| void MmkvLogger::log(const std::string& tag, const std::string& message) { | |||
| void MmkvLogger::log(const std::string& tag, const std::string& formatString, Args... args) { | |||
There was a problem hiding this comment.
same here, not a template.
we can't even template this because our implementation is in two differnt spots (ios and android). so we might not be able to support Args....
Avoid a string format unless logging is enabled.
Fixes #710.