From da6efbcaf1936341546d299b4540d7918b616ec0 Mon Sep 17 00:00:00 2001 From: Roman Gershman Date: Tue, 11 Sep 2018 00:13:32 +0300 Subject: [PATCH] Allow updating vmodule levels after vmodule was parsed, invalidate already cached site flags --- src/glog/vlog_is_on.h.in | 15 +++++++++++---- src/logging_unittest.cc | 18 ++++++++++++++++++ src/vlog_is_on.cc | 38 +++++++++++++++++++++++++++++++++++--- 3 files changed, 64 insertions(+), 7 deletions(-) diff --git a/src/glog/vlog_is_on.h.in b/src/glog/vlog_is_on.h.in index bcc812260..eab3e98e1 100644 --- a/src/glog/vlog_is_on.h.in +++ b/src/glog/vlog_is_on.h.in @@ -81,10 +81,10 @@ // parsing of --vmodule flag and/or SetVLOGLevel calls. #define VLOG_IS_ON(verboselevel) \ __extension__ \ - ({ static @ac_google_namespace@::int32* vlocal__ = NULL; \ + ({ static @ac_google_namespace@::SiteFlag vlocal__{NULL, NULL, 0, NULL}; \ @ac_google_namespace@::int32 verbose_level__ = (verboselevel); \ - (vlocal__ == NULL ? @ac_google_namespace@::InitVLOG3__(&vlocal__, &FLAGS_v, \ - __FILE__, verbose_level__) : *vlocal__ >= verbose_level__); \ + (vlocal__.level == NULL ? @ac_google_namespace@::InitVLOG3__(&vlocal__, &FLAGS_v, \ + __FILE__, verbose_level__) : *vlocal__.level >= verbose_level__); \ }) #else // GNU extensions not available, so we do not support --vmodule. @@ -105,6 +105,13 @@ extern GOOGLE_GLOG_DLL_DECL int SetVLOGLevel(const char* module_pattern, // Various declarations needed for VLOG_IS_ON above: ========================= +struct SiteFlag { + @ac_google_namespace@::int32* level; + const char* base_name; + size_t base_len; + SiteFlag* next; +}; + // Helper routine which determines the logging info for a particalur VLOG site. // site_flag is the address of the site-local pointer to the controlling // verbosity level @@ -114,7 +121,7 @@ extern GOOGLE_GLOG_DLL_DECL int SetVLOGLevel(const char* module_pattern, // We will return the return value for VLOG_IS_ON // and if possible set *site_flag appropriately. extern GOOGLE_GLOG_DLL_DECL bool InitVLOG3__( - @ac_google_namespace@::int32** site_flag, + @ac_google_namespace@::SiteFlag* site_flag, @ac_google_namespace@::int32* site_default, const char* fname, @ac_google_namespace@::int32 verbose_level); diff --git a/src/logging_unittest.cc b/src/logging_unittest.cc index edc7d35d7..31d9d0c7d 100644 --- a/src/logging_unittest.cc +++ b/src/logging_unittest.cc @@ -98,6 +98,7 @@ static void TestLogging(bool check_counts); static void TestRawLogging(); static void LogWithLevels(int v, int severity, bool err, bool alsoerr); static void TestLoggingLevels(); +static void TestVLogModule(); static void TestLogString(); static void TestLogSink(); static void TestLogToString(); @@ -219,6 +220,7 @@ int main(int argc, char **argv) { TestLogging(true); TestRawLogging(); TestLoggingLevels(); + TestVLogModule(); TestLogString(); TestLogSink(); TestLogToString(); @@ -449,6 +451,22 @@ void TestLoggingLevels() { LogWithLevels(1, GLOG_FATAL, false, true); } +int TestVlogHelper() { + if (VLOG_IS_ON(1)) { + return 1; + } + return 0; +} + +void TestVLogModule() { + int c = TestVlogHelper(); + EXPECT_EQ(0, c); + + EXPECT_EQ(0, SetVLOGLevel("logging_unittest", 1)); + c = TestVlogHelper(); + EXPECT_EQ(1, c); +} + TEST(DeathRawCHECK, logging) { ASSERT_DEATH(RAW_CHECK(false, "failure 1"), "RAW: Check false failed: failure 1"); diff --git a/src/vlog_is_on.cc b/src/vlog_is_on.cc index 4760366cd..a5278a2c1 100644 --- a/src/vlog_is_on.cc +++ b/src/vlog_is_on.cc @@ -125,6 +125,8 @@ static Mutex vmodule_lock; // Pointer to head of the VModuleInfo list. // It's a map from module pattern to logging level for those module(s). static VModuleInfo* vmodule_list = 0; +static SiteFlag* cached_site_list = 0; + // Boolean initialization flag. static bool inited_vmodule = false; @@ -190,6 +192,23 @@ int SetVLOGLevel(const char* module_pattern, int log_level) { info->vlog_level = log_level; info->next = vmodule_list; vmodule_list = info; + + SiteFlag** item_ptr = &cached_site_list; + SiteFlag* item = cached_site_list; + + // We traverse the list fully because the pattern can match several items + // from the list. + while (item) { + if (SafeFNMatch_(module_pattern, pattern_len, item->base_name, + item->base_len)) { + // Redirect the cached value to its module override. + item->level = &info->vlog_level; + *item_ptr = item->next; // Remove the item from the list. + } else { + item_ptr = &item->next; + } + item = *item_ptr; + } } } RAW_VLOG(1, "Set VLOG level for \"%s\" to %d", module_pattern, log_level); @@ -198,7 +217,7 @@ int SetVLOGLevel(const char* module_pattern, int log_level) { // NOTE: Individual VLOG statements cache the integer log level pointers. // NOTE: This function must not allocate memory or require any locks. -bool InitVLOG3__(int32** site_flag, int32* site_default, +bool InitVLOG3__(SiteFlag* site_flag, int32* level_default, const char* fname, int32 verbose_level) { MutexLock l(&vmodule_lock); bool read_vmodule_flag = inited_vmodule; @@ -211,7 +230,7 @@ bool InitVLOG3__(int32** site_flag, int32* site_default, int old_errno = errno; // site_default normally points to FLAGS_v - int32* site_flag_value = site_default; + int32* site_flag_value = level_default; // Get basename for file const char* base = strrchr(fname, '/'); @@ -244,7 +263,20 @@ bool InitVLOG3__(int32** site_flag, int32* site_default, ANNOTATE_BENIGN_RACE(site_flag, "*site_flag may be written by several threads," " but the value will be the same"); - if (read_vmodule_flag) *site_flag = site_flag_value; + if (read_vmodule_flag) { + site_flag->level = site_flag_value; + // If VLOG flag has been cached to the default site pointer, + // we want to add to the cached list in order to invalidate in case + // SetVModule is called afterwards with new modules. + // The performance penalty here is neglible, because InitVLOG3__ is called + // once per site. + if (site_flag_value == level_default && !site_flag->base_name) { + site_flag->base_name = base; + site_flag->base_len = base_length; + site_flag->next = cached_site_list; + cached_site_list = site_flag; + } + } // restore the errno in case something recoverable went wrong during // the initialization of the VLOG mechanism (see above note "protect the..")