Skip to content

Commit

Permalink
Take failure_message as const char* instead of string_view in LogMess…
Browse files Browse the repository at this point in the history
…ageFatal and friends.

This saves an instruction at every callsite and costs at most one strlen per process.  Okay, maybe a couple if two threads crash at once...

https://godbolt.org/z/vsqd35YaM

PiperOrigin-RevId: 698522200
Change-Id: I77735f0d650b029ca4b3e02b4f8027dfe0f3de4c
  • Loading branch information
suertreus authored and copybara-github committed Nov 20, 2024
1 parent b67caff commit 2fcebef
Show file tree
Hide file tree
Showing 10 changed files with 133 additions and 105 deletions.
1 change: 1 addition & 0 deletions absl/log/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ absl_cc_library(
DEPS
absl::config
absl::core_headers
absl::leak_check
absl::log_internal_nullguard
absl::log_internal_nullstream
absl::log_internal_strip
Expand Down
1 change: 1 addition & 0 deletions absl/log/internal/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ cc_library(
"//absl/base:config",
"//absl/base:core_headers",
"//absl/base:nullability",
"//absl/debugging:leak_check",
"//absl/strings",
],
)
Expand Down
49 changes: 27 additions & 22 deletions absl/log/internal/check_op.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,15 @@

#include "absl/log/internal/check_op.h"

#include <string.h>

#include <cstring>
#include <ostream>
#include <string>
#include <utility>

#include "absl/base/config.h"
#include "absl/base/nullability.h"
#include "absl/debugging/leak_check.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/string_view.h"

#ifdef _MSC_VER
Expand All @@ -26,18 +31,13 @@
#include <strings.h> // for strcasecmp, but msvc does not have this header
#endif

#include <sstream>
#include <string>

#include "absl/base/config.h"
#include "absl/strings/str_cat.h"

namespace absl {
ABSL_NAMESPACE_BEGIN
namespace log_internal {

#define ABSL_LOGGING_INTERNAL_DEFINE_MAKE_CHECK_OP_STRING(x) \
template std::string* MakeCheckOpString(x, x, const char*)
template absl::Nonnull<const char*> MakeCheckOpString( \
x, x, absl::Nonnull<const char*>)
ABSL_LOGGING_INTERNAL_DEFINE_MAKE_CHECK_OP_STRING(bool);
ABSL_LOGGING_INTERNAL_DEFINE_MAKE_CHECK_OP_STRING(int64_t);
ABSL_LOGGING_INTERNAL_DEFINE_MAKE_CHECK_OP_STRING(uint64_t);
Expand All @@ -53,7 +53,8 @@ ABSL_LOGGING_INTERNAL_DEFINE_MAKE_CHECK_OP_STRING(const unsigned char*);
ABSL_LOGGING_INTERNAL_DEFINE_MAKE_CHECK_OP_STRING(const void*);
#undef ABSL_LOGGING_INTERNAL_DEFINE_MAKE_CHECK_OP_STRING

CheckOpMessageBuilder::CheckOpMessageBuilder(const char* exprtext) {
CheckOpMessageBuilder::CheckOpMessageBuilder(
absl::Nonnull<const char*> exprtext) {
stream_ << exprtext << " (";
}

Expand All @@ -62,9 +63,10 @@ std::ostream& CheckOpMessageBuilder::ForVar2() {
return stream_;
}

std::string* CheckOpMessageBuilder::NewString() {
absl::Nonnull<const char*> CheckOpMessageBuilder::NewString() {
stream_ << ")";
return new std::string(stream_.str());
// There's no need to free this string since the process is crashing.
return absl::IgnoreLeak(new std::string(std::move(stream_).str()))->c_str();
}

void MakeCheckOpValueString(std::ostream& os, const char v) {
Expand Down Expand Up @@ -100,16 +102,19 @@ void MakeCheckOpValueString(std::ostream& os, const void* p) {
}

// Helper functions for string comparisons.
#define DEFINE_CHECK_STROP_IMPL(name, func, expected) \
std::string* Check##func##expected##Impl(const char* s1, const char* s2, \
const char* exprtext) { \
bool equal = s1 == s2 || (s1 && s2 && !func(s1, s2)); \
if (equal == expected) { \
return nullptr; \
} else { \
return new std::string( \
absl::StrCat(exprtext, " (", s1, " vs. ", s2, ")")); \
} \
#define DEFINE_CHECK_STROP_IMPL(name, func, expected) \
absl::Nullable<const char*> Check##func##expected##Impl( \
absl::Nullable<const char*> s1, absl::Nullable<const char*> s2, \
absl::Nonnull<const char*> exprtext) { \
bool equal = s1 == s2 || (s1 && s2 && !func(s1, s2)); \
if (equal == expected) { \
return nullptr; \
} else { \
/* There's no need to free this string since the process is crashing. */ \
return absl::IgnoreLeak(new std::string(absl::StrCat(exprtext, " (", s1, \
" vs. ", s2, ")"))) \
->c_str(); \
} \
}
DEFINE_CHECK_STROP_IMPL(CHECK_STREQ, strcmp, true)
DEFINE_CHECK_STROP_IMPL(CHECK_STRNE, strcmp, false)
Expand Down
117 changes: 63 additions & 54 deletions absl/log/internal/check_op.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,44 +63,43 @@
#endif

#define ABSL_LOG_INTERNAL_CHECK_OP(name, op, val1, val1_text, val2, val2_text) \
while (::std::string* absl_log_internal_check_op_result \
while (absl::Nullable<const char*> absl_log_internal_check_op_result \
ABSL_LOG_INTERNAL_ATTRIBUTE_UNUSED_IF_STRIP_LOG = \
::absl::log_internal::name##Impl( \
::absl::log_internal::GetReferenceableValue(val1), \
::absl::log_internal::GetReferenceableValue(val2), \
ABSL_LOG_INTERNAL_STRIP_STRING_LITERAL( \
val1_text " " #op " " val2_text))) \
ABSL_LOG_INTERNAL_CONDITION_FATAL(STATELESS, true) \
ABSL_LOG_INTERNAL_CHECK(*absl_log_internal_check_op_result).InternalStream()
#define ABSL_LOG_INTERNAL_QCHECK_OP(name, op, val1, val1_text, val2, \
val2_text) \
while (::std::string* absl_log_internal_qcheck_op_result = \
::absl::log_internal::name##Impl( \
::absl::log_internal::GetReferenceableValue(val1), \
::absl::log_internal::GetReferenceableValue(val2), \
ABSL_LOG_INTERNAL_STRIP_STRING_LITERAL( \
val1_text " " #op " " val2_text))) \
ABSL_LOG_INTERNAL_CONDITION_QFATAL(STATELESS, true) \
ABSL_LOG_INTERNAL_QCHECK(*absl_log_internal_qcheck_op_result).InternalStream()
ABSL_LOG_INTERNAL_CHECK(absl_log_internal_check_op_result).InternalStream()
#define ABSL_LOG_INTERNAL_QCHECK_OP(name, op, val1, val1_text, val2, \
val2_text) \
while (absl::Nullable<const char*> absl_log_internal_qcheck_op_result = \
::absl::log_internal::name##Impl( \
::absl::log_internal::GetReferenceableValue(val1), \
::absl::log_internal::GetReferenceableValue(val2), \
ABSL_LOG_INTERNAL_STRIP_STRING_LITERAL( \
val1_text " " #op " " val2_text))) \
ABSL_LOG_INTERNAL_CONDITION_QFATAL(STATELESS, true) \
ABSL_LOG_INTERNAL_QCHECK(absl_log_internal_qcheck_op_result).InternalStream()
#define ABSL_LOG_INTERNAL_CHECK_STROP(func, op, expected, s1, s1_text, s2, \
s2_text) \
while (::std::string* absl_log_internal_check_strop_result = \
while (absl::Nullable<const char*> absl_log_internal_check_strop_result = \
::absl::log_internal::Check##func##expected##Impl( \
(s1), (s2), \
ABSL_LOG_INTERNAL_STRIP_STRING_LITERAL(s1_text " " #op \
" " s2_text))) \
ABSL_LOG_INTERNAL_CONDITION_FATAL(STATELESS, true) \
ABSL_LOG_INTERNAL_CHECK(*absl_log_internal_check_strop_result) \
.InternalStream()
ABSL_LOG_INTERNAL_CHECK(absl_log_internal_check_strop_result).InternalStream()
#define ABSL_LOG_INTERNAL_QCHECK_STROP(func, op, expected, s1, s1_text, s2, \
s2_text) \
while (::std::string* absl_log_internal_qcheck_strop_result = \
while (absl::Nullable<const char*> absl_log_internal_qcheck_strop_result = \
::absl::log_internal::Check##func##expected##Impl( \
(s1), (s2), \
ABSL_LOG_INTERNAL_STRIP_STRING_LITERAL(s1_text " " #op \
" " s2_text))) \
ABSL_LOG_INTERNAL_CONDITION_QFATAL(STATELESS, true) \
ABSL_LOG_INTERNAL_QCHECK(*absl_log_internal_qcheck_strop_result) \
ABSL_LOG_INTERNAL_QCHECK(absl_log_internal_qcheck_strop_result) \
.InternalStream()
// This one is tricky:
// * We must evaluate `val` exactly once, yet we need to do two things with it:
Expand All @@ -127,7 +126,8 @@
// strip the call to stringify the non-ok `Status` as long as we don't log it;
// dropping the `Status`'s message text is out of scope.
#define ABSL_LOG_INTERNAL_CHECK_OK(val, val_text) \
for (::std::pair<const ::absl::Status*, ::std::string*> \
for (::std::pair<absl::Nonnull<const ::absl::Status*>, \
absl::Nullable<const char*>> \
absl_log_internal_check_ok_goo; \
absl_log_internal_check_ok_goo.first = \
::absl::log_internal::AsStatus(val), \
Expand All @@ -140,10 +140,11 @@
" is OK")), \
!ABSL_PREDICT_TRUE(absl_log_internal_check_ok_goo.first->ok());) \
ABSL_LOG_INTERNAL_CONDITION_FATAL(STATELESS, true) \
ABSL_LOG_INTERNAL_CHECK(*absl_log_internal_check_ok_goo.second) \
ABSL_LOG_INTERNAL_CHECK(absl_log_internal_check_ok_goo.second) \
.InternalStream()
#define ABSL_LOG_INTERNAL_QCHECK_OK(val, val_text) \
for (::std::pair<const ::absl::Status*, ::std::string*> \
for (::std::pair<absl::Nonnull<const ::absl::Status*>, \
absl::Nullable<const char*>> \
absl_log_internal_qcheck_ok_goo; \
absl_log_internal_qcheck_ok_goo.first = \
::absl::log_internal::AsStatus(val), \
Expand All @@ -156,7 +157,7 @@
" is OK")), \
!ABSL_PREDICT_TRUE(absl_log_internal_qcheck_ok_goo.first->ok());) \
ABSL_LOG_INTERNAL_CONDITION_QFATAL(STATELESS, true) \
ABSL_LOG_INTERNAL_QCHECK(*absl_log_internal_qcheck_ok_goo.second) \
ABSL_LOG_INTERNAL_QCHECK(absl_log_internal_qcheck_ok_goo.second) \
.InternalStream()

namespace absl {
Expand All @@ -167,7 +168,7 @@ template <typename T>
class StatusOr;

namespace status_internal {
ABSL_ATTRIBUTE_PURE_FUNCTION absl::Nonnull<std::string*> MakeCheckFailString(
ABSL_ATTRIBUTE_PURE_FUNCTION absl::Nonnull<const char*> MakeCheckFailString(
absl::Nonnull<const absl::Status*> status,
absl::Nonnull<const char*> prefix);
} // namespace status_internal
Expand All @@ -177,9 +178,11 @@ namespace log_internal {
// Convert a Status or a StatusOr to its underlying status value.
//
// (This implementation does not require a dep on absl::Status to work.)
inline const absl::Status* AsStatus(const absl::Status& s) { return &s; }
inline absl::Nonnull<const absl::Status*> AsStatus(const absl::Status& s) {
return &s;
}
template <typename T>
const absl::Status* AsStatus(const absl::StatusOr<T>& s) {
absl::Nonnull<const absl::Status*> AsStatus(const absl::StatusOr<T>& s) {
return &s.status();
}

Expand All @@ -188,14 +191,14 @@ const absl::Status* AsStatus(const absl::StatusOr<T>& s) {
class CheckOpMessageBuilder final {
public:
// Inserts `exprtext` and ` (` to the stream.
explicit CheckOpMessageBuilder(const char* exprtext);
explicit CheckOpMessageBuilder(absl::Nonnull<const char*> exprtext);
~CheckOpMessageBuilder() = default;
// For inserting the first variable.
std::ostream& ForVar1() { return stream_; }
// For inserting the second variable (adds an intermediate ` vs. `).
std::ostream& ForVar2();
// Get the result (inserts the closing `)`).
std::string* NewString();
absl::Nonnull<const char*> NewString();

private:
std::ostringstream stream_;
Expand Down Expand Up @@ -338,11 +341,12 @@ using CheckOpStreamType = decltype(detect_specialization::Detect<T>(0));

// Build the error message string. Specify no inlining for code size.
template <typename T1, typename T2>
ABSL_ATTRIBUTE_RETURNS_NONNULL std::string* MakeCheckOpString(
T1 v1, T2 v2, const char* exprtext) ABSL_ATTRIBUTE_NOINLINE;
ABSL_ATTRIBUTE_RETURNS_NONNULL absl::Nonnull<const char*> MakeCheckOpString(
T1 v1, T2 v2, absl::Nonnull<const char*> exprtext) ABSL_ATTRIBUTE_NOINLINE;

template <typename T1, typename T2>
std::string* MakeCheckOpString(T1 v1, T2 v2, const char* exprtext) {
absl::Nonnull<const char*> MakeCheckOpString(
T1 v1, T2 v2, absl::Nonnull<const char*> exprtext) {
CheckOpMessageBuilder comb(exprtext);
MakeCheckOpValueString(comb.ForVar1(), v1);
MakeCheckOpValueString(comb.ForVar2(), v2);
Expand All @@ -352,7 +356,8 @@ std::string* MakeCheckOpString(T1 v1, T2 v2, const char* exprtext) {
// Add a few commonly used instantiations as extern to reduce size of objects
// files.
#define ABSL_LOG_INTERNAL_DEFINE_MAKE_CHECK_OP_STRING_EXTERN(x) \
extern template std::string* MakeCheckOpString(x, x, const char*)
extern template absl::Nonnull<const char*> MakeCheckOpString( \
x, x, absl::Nonnull<const char*>)
ABSL_LOG_INTERNAL_DEFINE_MAKE_CHECK_OP_STRING_EXTERN(bool);
ABSL_LOG_INTERNAL_DEFINE_MAKE_CHECK_OP_STRING_EXTERN(int64_t);
ABSL_LOG_INTERNAL_DEFINE_MAKE_CHECK_OP_STRING_EXTERN(uint64_t);
Expand All @@ -376,7 +381,7 @@ ABSL_LOG_INTERNAL_DEFINE_MAKE_CHECK_OP_STRING_EXTERN(const void*);
((::absl::LogSeverity::kFatal >= \
static_cast<::absl::LogSeverity>(ABSL_MIN_LOG_LEVEL)) \
? MakeCheckOpString<U1, U2>(v1, v2, exprtext) \
: new std::string())
: "")
#else
#define ABSL_LOG_INTERNAL_CHECK_OP_IMPL_RESULT(U1, U2, v1, v2, exprtext) \
MakeCheckOpString<U1, U2>(v1, v2, exprtext)
Expand All @@ -388,17 +393,17 @@ ABSL_LOG_INTERNAL_DEFINE_MAKE_CHECK_OP_STRING_EXTERN(const void*);
// type.
#define ABSL_LOG_INTERNAL_CHECK_OP_IMPL(name, op) \
template <typename T1, typename T2> \
inline constexpr ::std::string* name##Impl(const T1& v1, const T2& v2, \
const char* exprtext) { \
inline constexpr absl::Nullable<const char*> name##Impl( \
const T1& v1, const T2& v2, absl::Nonnull<const char*> exprtext) { \
using U1 = CheckOpStreamType<T1>; \
using U2 = CheckOpStreamType<T2>; \
return ABSL_PREDICT_TRUE(v1 op v2) \
? nullptr \
: ABSL_LOG_INTERNAL_CHECK_OP_IMPL_RESULT(U1, U2, U1(v1), \
U2(v2), exprtext); \
} \
inline constexpr ::std::string* name##Impl(int v1, int v2, \
const char* exprtext) { \
inline constexpr absl::Nullable<const char*> name##Impl( \
int v1, int v2, absl::Nonnull<const char*> exprtext) { \
return name##Impl<int, int>(v1, v2, exprtext); \
}

Expand All @@ -411,21 +416,27 @@ ABSL_LOG_INTERNAL_CHECK_OP_IMPL(Check_GT, >)
#undef ABSL_LOG_INTERNAL_CHECK_OP_IMPL_RESULT
#undef ABSL_LOG_INTERNAL_CHECK_OP_IMPL

std::string* CheckstrcmptrueImpl(const char* s1, const char* s2,
const char* exprtext);
std::string* CheckstrcmpfalseImpl(const char* s1, const char* s2,
const char* exprtext);
std::string* CheckstrcasecmptrueImpl(const char* s1, const char* s2,
const char* exprtext);
std::string* CheckstrcasecmpfalseImpl(const char* s1, const char* s2,
const char* exprtext);
absl::Nullable<const char*> CheckstrcmptrueImpl(
absl::Nullable<const char*> s1, absl::Nullable<const char*> s2,
absl::Nonnull<const char*> exprtext);
absl::Nullable<const char*> CheckstrcmpfalseImpl(
absl::Nullable<const char*> s1, absl::Nullable<const char*> s2,
absl::Nonnull<const char*> exprtext);
absl::Nullable<const char*> CheckstrcasecmptrueImpl(
absl::Nullable<const char*> s1, absl::Nullable<const char*> s2,
absl::Nonnull<const char*> exprtext);
absl::Nullable<const char*> CheckstrcasecmpfalseImpl(
absl::Nullable<const char*> s1, absl::Nullable<const char*> s2,
absl::Nonnull<const char*> exprtext);

// `CHECK_EQ` and friends want to pass their arguments by reference, however
// this winds up exposing lots of cases where people have defined and
// initialized static const data members but never declared them (i.e. in a .cc
// file), meaning they are not referenceable. This function avoids that problem
// for integers (the most common cases) by overloading for every primitive
// integer type, even the ones we discourage, and returning them by value.
// NOLINTBEGIN(runtime/int)
// NOLINTBEGIN(google-runtime-int)
template <typename T>
inline constexpr const T& GetReferenceableValue(const T& t) {
return t;
Expand All @@ -435,27 +446,25 @@ inline constexpr unsigned char GetReferenceableValue(unsigned char t) {
return t;
}
inline constexpr signed char GetReferenceableValue(signed char t) { return t; }
inline constexpr short GetReferenceableValue(short t) { return t; } // NOLINT
inline constexpr unsigned short GetReferenceableValue( // NOLINT
unsigned short t) { // NOLINT
inline constexpr short GetReferenceableValue(short t) { return t; }
inline constexpr unsigned short GetReferenceableValue(unsigned short t) {
return t;
}
inline constexpr int GetReferenceableValue(int t) { return t; }
inline constexpr unsigned int GetReferenceableValue(unsigned int t) {
return t;
}
inline constexpr long GetReferenceableValue(long t) { return t; } // NOLINT
inline constexpr unsigned long GetReferenceableValue( // NOLINT
unsigned long t) { // NOLINT
return t;
}
inline constexpr long long GetReferenceableValue(long long t) { // NOLINT
inline constexpr long GetReferenceableValue(long t) { return t; }
inline constexpr unsigned long GetReferenceableValue(unsigned long t) {
return t;
}
inline constexpr unsigned long long GetReferenceableValue( // NOLINT
unsigned long long t) { // NOLINT
inline constexpr long long GetReferenceableValue(long long t) { return t; }
inline constexpr unsigned long long GetReferenceableValue(
unsigned long long t) {
return t;
}
// NOLINTEND(google-runtime-int)
// NOLINTEND(runtime/int)

} // namespace log_internal
ABSL_NAMESPACE_END
Expand Down
Loading

0 comments on commit 2fcebef

Please sign in to comment.