Deduplicate VLogSite instances per source file and shrink VLOG callsite codegen. Previously, every VLOG callsite created its own static VLogSite instance even though VLogSite registration and verbosity updates depend only on the source file path. To reduce binary size and runtime memory: 1. Deduplicate VLogSite instances per file. 2. Move the atomic verbosity level to the start of VLogSite so the fast path accesses it at offset zero. 3. Reload the verbosity level inside the out-of-line slow-path helpers and avoiding passing the stale level. PiperOrigin-RevId: 1008379641
diff --git a/absl/log/absl_vlog_is_on.h b/absl/log/absl_vlog_is_on.h index 1a6792c..a099b97 100644 --- a/absl/log/absl_vlog_is_on.h +++ b/absl/log/absl_vlog_is_on.h
@@ -55,13 +55,12 @@ // Resetting --vmodule will override all previous modifications to `--vmodule`, // including via SetVLogLevel. +// SKIP_ABSL_INLINE_NAMESPACE_CHECK + #ifndef ABSL_LOG_ABSL_VLOG_IS_ON_H_ #define ABSL_LOG_ABSL_VLOG_IS_ON_H_ -#include "absl/base/attributes.h" -#include "absl/base/config.h" #include "absl/log/internal/vlog_config.h" // IWYU pragma: export -#include "absl/strings/string_view.h" // This is expanded at the callsite to allow the compiler to optimize // always-false cases out of the build. @@ -74,20 +73,17 @@ #define ABSL_LOG_INTERNAL_MAX_LOG_VERBOSITY_CHECK(x) #endif -// Each ABSL_VLOG_IS_ON call site gets its own VLogSite that registers with the -// global linked list of sites to asynchronously update its verbosity level on -// changes to --v or --vmodule. The verbosity can also be set by manually -// calling SetVLogLevel. +// Each source file's ABSL_VLOG_IS_ON call sites share a single VLogSite that +// registers with the global linked list of sites to asynchronously update its +// verbosity level on changes to --v or --vmodule. The verbosity can also be set +// by manually calling SetVLogLevel. // // ABSL_VLOG_IS_ON is not async signal safe, but it is guaranteed not to // allocate new memory. -#define ABSL_VLOG_IS_ON(verbose_level) \ - (ABSL_LOG_INTERNAL_MAX_LOG_VERBOSITY_CHECK(verbose_level)[]() \ - ->::absl::log_internal::VLogSite * \ - { \ - ABSL_CONST_INIT static ::absl::log_internal::VLogSite site(__FILE__); \ - return &site; \ - }() \ - ->IsEnabled(verbose_level)) +#define ABSL_VLOG_IS_ON(verbose_level) \ + (ABSL_LOG_INTERNAL_MAX_LOG_VERBOSITY_CHECK( \ + verbose_level)::absl::log_internal::GetVLogSiteHolder([] { \ + return __FILE__; \ + }).site.IsEnabled(verbose_level)) #endif // ABSL_LOG_ABSL_VLOG_IS_ON_H_
diff --git a/absl/log/internal/vlog_config.cc b/absl/log/internal/vlog_config.cc index 79eb7df..304aca4 100644 --- a/absl/log/internal/vlog_config.cc +++ b/absl/log/internal/vlog_config.cc
@@ -64,23 +64,21 @@ } // namespace -bool VLogSite::SlowIsEnabled(int stale_v, int level) { +bool VLogSite::SlowIsEnabled(int level) { + int stale_v = v_.load(std::memory_order_relaxed); if (ABSL_PREDICT_TRUE(stale_v != kUninitialized)) { - // Because of the prerequisites to this function, we know that stale_v is - // either uninitialized or >= level. If it's not uninitialized, that means - // it must be >= level, thus we should log. - return true; + return stale_v >= level; } stale_v = log_internal::RegisterAndInitialize(this); return ABSL_PREDICT_FALSE(stale_v >= level); } -bool VLogSite::SlowIsEnabled0(int stale_v) { return SlowIsEnabled(stale_v, 0); } -bool VLogSite::SlowIsEnabled1(int stale_v) { return SlowIsEnabled(stale_v, 1); } -bool VLogSite::SlowIsEnabled2(int stale_v) { return SlowIsEnabled(stale_v, 2); } -bool VLogSite::SlowIsEnabled3(int stale_v) { return SlowIsEnabled(stale_v, 3); } -bool VLogSite::SlowIsEnabled4(int stale_v) { return SlowIsEnabled(stale_v, 4); } -bool VLogSite::SlowIsEnabled5(int stale_v) { return SlowIsEnabled(stale_v, 5); } +bool VLogSite::SlowIsEnabled0() { return SlowIsEnabled(0); } +bool VLogSite::SlowIsEnabled1() { return SlowIsEnabled(1); } +bool VLogSite::SlowIsEnabled2() { return SlowIsEnabled(2); } +bool VLogSite::SlowIsEnabled3() { return SlowIsEnabled(3); } +bool VLogSite::SlowIsEnabled4() { return SlowIsEnabled(4); } +bool VLogSite::SlowIsEnabled5() { return SlowIsEnabled(5); } namespace { struct VModuleInfo final {
diff --git a/absl/log/internal/vlog_config.h b/absl/log/internal/vlog_config.h index f65eb5d..c0d08c6 100644 --- a/absl/log/internal/vlog_config.h +++ b/absl/log/internal/vlog_config.h
@@ -27,10 +27,12 @@ // IWYU pragma: private, include "absl/log/log.h" #include <atomic> +#include <cstddef> #include <cstdint> #include <functional> #include <limits> #include <type_traits> +#include <utility> #include "absl/base/attributes.h" #include "absl/base/config.h" @@ -49,19 +51,21 @@ int RegisterAndInitialize(VLogSite* absl_nonnull v); void UpdateVLogSites(); -// Represents a unique callsite for a `VLOG()` or `VLOG_IS_ON()` call. +// Represents the `--vmodule` state for a source file. // -// Libraries that provide `VLOG`-like functionality should use this to -// efficiently handle --vmodule. +// When the file name is known at the callsite (`__FILE__`), prefer +// `VLOG_IS_ON(level)` so all callsites in the same file share a single +// `VLogSite`. Construct `VLogSite` directly only when the file name is +// determined at runtime (e.g. from `absl::SourceLocation` or language +// bindings). // -// VLogSite objects must not be destroyed until the program exits. Doing so will -// probably yield nasty segfaults in VLogSiteManager::UpdateLogSites(). The -// recommendation is to make all such objects function-local statics. +// `VLogSite` objects must not be destroyed until the program exits; doing so +// will corrupt the global `VLogSite` list in `UpdateVLogSites()`. class VLogSite final { public: // `f` must not be destroyed until the program exits. explicit constexpr VLogSite(const char* absl_nonnull f) - : file_(f), v_(kUninitialized), next_(nullptr) {} + : v_(kUninitialized), file_(f), next_(nullptr) {} VLogSite(const VLogSite&) = delete; VLogSite& operator=(const VLogSite&) = delete; @@ -81,15 +85,15 @@ // of code space by special-casing for a few common levels. #if ABSL_HAVE_BUILTIN(__builtin_constant_p) || defined(__GNUC__) if (__builtin_constant_p(level)) { - if (level == 0) return SlowIsEnabled0(stale_v); - if (level == 1) return SlowIsEnabled1(stale_v); - if (level == 2) return SlowIsEnabled2(stale_v); - if (level == 3) return SlowIsEnabled3(stale_v); - if (level == 4) return SlowIsEnabled4(stale_v); - if (level == 5) return SlowIsEnabled5(stale_v); + if (level == 0) return SlowIsEnabled0(); + if (level == 1) return SlowIsEnabled1(); + if (level == 2) return SlowIsEnabled2(); + if (level == 3) return SlowIsEnabled3(); + if (level == 4) return SlowIsEnabled4(); + if (level == 5) return SlowIsEnabled5(); } #endif - return SlowIsEnabled(stale_v, level); + return SlowIsEnabled(level); } private: @@ -99,30 +103,67 @@ static constexpr int kUninitialized = (std::numeric_limits<int>::max)(); // SlowIsEnabled performs slower checks to determine whether a log site is - // enabled. Because it is expected to be called somewhat rarely - // (comparatively), it is not inlined to save on code size. + // enabled, registering and initializing the site if needed. Because it is + // expected to be called somewhat rarely (comparatively), it is not inlined to + // save on code size. // - // Prerequisites to calling SlowIsEnabled: - // 1) stale_v is uninitialized OR - // 2) stale_v is initialized and >= level (meaning we must log). // Takes locks but does not allocate memory. - ABSL_ATTRIBUTE_NOINLINE - bool SlowIsEnabled(int stale_v, int level); - ABSL_ATTRIBUTE_NOINLINE bool SlowIsEnabled0(int stale_v); - ABSL_ATTRIBUTE_NOINLINE bool SlowIsEnabled1(int stale_v); - ABSL_ATTRIBUTE_NOINLINE bool SlowIsEnabled2(int stale_v); - ABSL_ATTRIBUTE_NOINLINE bool SlowIsEnabled3(int stale_v); - ABSL_ATTRIBUTE_NOINLINE bool SlowIsEnabled4(int stale_v); - ABSL_ATTRIBUTE_NOINLINE bool SlowIsEnabled5(int stale_v); + ABSL_ATTRIBUTE_NOINLINE bool SlowIsEnabled(int level); + ABSL_ATTRIBUTE_NOINLINE bool SlowIsEnabled0(); + ABSL_ATTRIBUTE_NOINLINE bool SlowIsEnabled1(); + ABSL_ATTRIBUTE_NOINLINE bool SlowIsEnabled2(); + ABSL_ATTRIBUTE_NOINLINE bool SlowIsEnabled3(); + ABSL_ATTRIBUTE_NOINLINE bool SlowIsEnabled4(); + ABSL_ATTRIBUTE_NOINLINE bool SlowIsEnabled5(); + // `v_` is kept at offset 0 so the fast path accesses `site` without a struct + // field displacement. // This object is too size-sensitive to use absl::string_view. - const char* absl_nonnull const file_; std::atomic<int> v_; + const char* absl_nonnull const file_; std::atomic<VLogSite*> next_; }; static_assert(std::is_trivially_destructible_v<VLogSite>, "VLogSite must be trivially destructible"); +#if ABSL_HAVE_ATTRIBUTE(visibility) +#define ABSL_LOG_INTERNAL_HIDDEN __attribute__((visibility("hidden"))) +#else +#define ABSL_LOG_INTERNAL_HIDDEN +#endif +// Prevents the compiler from padding the file name array to 16 bytes. +#if ABSL_HAVE_ATTRIBUTE(aligned) +#define ABSL_LOG_INTERNAL_ALIGNED_1 __attribute__((aligned(1))) +#else +#define ABSL_LOG_INTERNAL_ALIGNED_1 +#endif + +// TODO: b/576087803 - Migrate to a C++20 class NTTP once Abseil requires C++20. +// Hidden visibility: +// 1. Keeps `site` and `kFile` out of `.dynsym`/`.dynstr` in shared libraries. +// 2. Allows direct PC-relative addressing instead of GOT indirection in PIC. +template <char... Cs> +struct ABSL_LOG_INTERNAL_HIDDEN VLogSiteHolder { + ABSL_LOG_INTERNAL_ALIGNED_1 static constexpr char kFile[sizeof...(Cs) + 1] = + {Cs..., '\0'}; + ABSL_CONST_INIT static inline VLogSite site{kFile}; +}; + +#undef ABSL_LOG_INTERNAL_ALIGNED_1 +#undef ABSL_LOG_INTERNAL_HIDDEN + +template <typename F, size_t... Is> +ABSL_ATTRIBUTE_ALWAYS_INLINE inline auto GetVLogSiteHolderImpl( + F f, std::index_sequence<Is...>) { + return VLogSiteHolder<f()[Is]...>{}; +} + +template <typename F> +ABSL_ATTRIBUTE_ALWAYS_INLINE inline auto GetVLogSiteHolder(F f) { + return GetVLogSiteHolderImpl( + f, std::make_index_sequence<absl::string_view(f()).size()>{}); +} + // Returns the current verbose log level of `file`. // Does not allocate memory. int VLogLevel(absl::string_view file);
diff --git a/absl/log/vlog_is_on.h b/absl/log/vlog_is_on.h index 67c6fb3..7ecd6fa 100644 --- a/absl/log/vlog_is_on.h +++ b/absl/log/vlog_is_on.h
@@ -55,15 +55,17 @@ // Resetting --vmodule will override all previous modifications to `--vmodule`, // including via SetVLogLevel. +// SKIP_ABSL_INLINE_NAMESPACE_CHECK + #ifndef ABSL_LOG_VLOG_IS_ON_H_ #define ABSL_LOG_VLOG_IS_ON_H_ #include "absl/log/absl_vlog_is_on.h" // IWYU pragma: export -// Each VLOG_IS_ON call site gets its own VLogSite that registers with the -// global linked list of sites to asynchronously update its verbosity level on -// changes to --v or --vmodule. The verbosity can also be set by manually -// calling SetVLogLevel. +// Each source file's VLOG_IS_ON call sites share a single VLogSite that +// registers with the global linked list of sites to asynchronously update its +// verbosity level on changes to --v or --vmodule. The verbosity can also be set +// by manually calling SetVLogLevel. // // VLOG_IS_ON is not async signal safe, but it is guaranteed not to allocate // new memory.
diff --git a/absl/log/vlog_is_on_test.cc b/absl/log/vlog_is_on_test.cc index d523781..81ef7b3 100644 --- a/absl/log/vlog_is_on_test.cc +++ b/absl/log/vlog_is_on_test.cc
@@ -231,4 +231,28 @@ VLOG(4) << "dfgh"; } +// Verifies that VLOG_IS_ON callsites in the same file share a single +// VLogSite instance, while callsites in different files get distinct VLogSite +// instances. +TEST_F(VLogIsOnTest, DeduplicatesVLogSitePerFile) { + if (MaxLogVerbosity().has_value() || MinLogLevel().has_value()) { + GTEST_SKIP(); + } + + using ::absl::log_internal::GetVLogSiteHolder; + // Not `decltype(GetVLogSiteHolder(...))::site`: lambdas may not appear in + // unevaluated operands before C++20. + auto* site1 = &GetVLogSiteHolder([] { return __FILE__; }).site; + auto* site2 = &GetVLogSiteHolder([] { return __FILE__; }).site; + auto* other_file_site = + &GetVLogSiteHolder([] { return "other_file.cc"; }).site; + EXPECT_EQ(site1, site2); + EXPECT_NE(site1, other_file_site); + + absl::SetVLogLevel("vlog_is_on_test", 2); + EXPECT_TRUE(VLOG_IS_ON(1)); + EXPECT_TRUE(VLOG_IS_ON(2)); + EXPECT_FALSE(VLOG_IS_ON(3)); +} + } // namespace