From 97bb53e32dd830b2d44b9220b3d7a3aa2e39c3f6 Mon Sep 17 00:00:00 2001 From: Aaron Gyes Date: Tue, 28 Sep 2021 23:31:47 -0700 Subject: [PATCH] Add likely() and unlikely() for our assertions Allows the compiler to know our bespoke assert functions are cold paths. This would normally occur somehow for real assert(). Assembly does appear it will save some branches. Also don't worry about NDEBUG (This doesn't matter because we rolled our own assert functions. Thanks @zanchey.) --- CMakeLists.txt | 3 --- src/common.cpp | 10 +++++----- src/common.h | 11 ++++++++++- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 22bfeb4cd..5eeea4db5 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -19,9 +19,6 @@ set(DEFAULT_BUILD_TYPE "RelWithDebInfo") # Generate Xcode schemas (but not for tests). set(CMAKE_XCODE_GENERATE_SCHEME 1) -# Use the default flags (#6296) but undefine NDEBUG so that asserts remain enabled. -add_definitions(-UNDEBUG) - if(NOT CMAKE_BUILD_TYPE AND NOT CMAKE_CONFIGURATION_TYPES) message(STATUS "Setting build type to default '${DEFAULT_BUILD_TYPE}'") set(CMAKE_BUILD_TYPE "${DEFAULT_BUILD_TYPE}") diff --git a/src/common.cpp b/src/common.cpp index 82390c9d7..391aa795b 100644 --- a/src/common.cpp +++ b/src/common.cpp @@ -1780,7 +1780,7 @@ void restore_term_foreground_process_group_for_exit() { bool is_main_thread() { return thread_id() == 1; } void assert_is_main_thread(const char *who) { - if (!is_main_thread() && !thread_asserts_cfg_for_testing) { + if (!likely(is_main_thread()) && !unlikely(thread_asserts_cfg_for_testing)) { FLOGF(error, L"%s called off of main thread.", who); FLOGF(error, L"Break on debug_thread_error to debug."); debug_thread_error(); @@ -1788,7 +1788,7 @@ void assert_is_main_thread(const char *who) { } void assert_is_not_forked_child(const char *who) { - if (is_forked_child()) { + if (unlikely(is_forked_child())) { FLOGF(error, L"%s called in a forked child.", who); FLOG(error, L"Break on debug_thread_error to debug."); debug_thread_error(); @@ -1796,7 +1796,7 @@ void assert_is_not_forked_child(const char *who) { } void assert_is_background_thread(const char *who) { - if (is_main_thread() && !thread_asserts_cfg_for_testing) { + if (unlikely(is_main_thread()) && !unlikely(thread_asserts_cfg_for_testing)) { FLOGF(error, L"%s called on the main thread (may block!).", who); FLOG(error, L"Break on debug_thread_error to debug."); debug_thread_error(); @@ -1806,7 +1806,7 @@ void assert_is_background_thread(const char *who) { void assert_is_locked(std::mutex &mutex, const char *who, const char *caller) { // Note that std::mutex.try_lock() is allowed to return false when the mutex isn't // actually locked; fortunately we are checking the opposite so we're safe. - if (mutex.try_lock()) { + if (unlikely(mutex.try_lock())) { FLOGF(error, L"%s is not locked when it should be in '%s'", who, caller); FLOG(error, L"Break on debug_thread_error to debug."); debug_thread_error(); @@ -1843,7 +1843,7 @@ void redirect_tty_output() { /// Display a failed assertion message, dump a stack trace if possible, then die. [[noreturn]] void __fish_assert(const char *msg, const char *file, size_t line, int error) { - if (error) { + if (unlikely(error)) { FLOGF(error, L"%s:%zu: failed assertion: %s: errno %d (%s)", file, line, msg, error, std::strerror(error)); } else { diff --git a/src/common.h b/src/common.h index 28567d2e8..500be8780 100644 --- a/src/common.h +++ b/src/common.h @@ -222,7 +222,7 @@ extern const wcstring g_empty_string; #define DIE_ON_FAILURE(e) \ do { \ int status = e; \ - if (status != 0) { \ + if (unlikely(status != 0)) { \ __fish_assert(#e, __FILE__, __LINE__, status); \ } \ } while (0) @@ -308,6 +308,15 @@ void wcs2string_appending(const wchar_t *in, size_t len, std::string *receiver); #define TESTS_PROGRAM_NAME L"(ignore)" bool should_suppress_stderr_for_tests(); +/// Branch prediction hints. Idea borrowed from Linux kernel. Just used for asserts. +#if __has_builtin(__builtin_expect) +#define likely(x) __builtin_expect(!!(x), 1) +#define unlikely(x) __builtin_expect(!!(x), 0) +#else +#define likely(x) (x) +#define unlikely(x) (x) +#endif + void assert_is_main_thread(const char *who); #define ASSERT_IS_MAIN_THREAD_TRAMPOLINE(x) assert_is_main_thread(x) #define ASSERT_IS_MAIN_THREAD() ASSERT_IS_MAIN_THREAD_TRAMPOLINE(__FUNCTION__)