Added support for filtering on the entire pathname to --vmodule.

Also cleaned up spurious warning when --vmodule is used but --v is not.

Removed slow perf unittest from vlog_unittest.cc.

BUG=61123
TEST=New vlog unittests

Review URL: http://codereview.chromium.org/4140007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@64346 0039d316-1c4b-4281-b951-d872f2087c98

5 files changed, 111 insertions, 90 deletions

diff --git a/base/base_switches.cc b/base/base_switches.cc
index 49c6487..d907a3a 100644
--- a/base/base_switches.cc
+++ b/base/base_switches.cc
@@ -39,6 +39,11 @@ const char kV[]                             = "v";
 // given by --v.  E.g. "my_module=2,foo*=3" would change the logging
 // level for all code in source files "my_module.*" and "foo*.*"
 // ("-inl" suffixes are also disregarded for this matching).
+//
+// Any pattern containing a forward or backward slash will be tested
+// against the whole pathname and not just the module.  E.g.,
+// "*/foo/bar/*=2" would change the logging level for all code in
+// source files under a "foo/bar" directory.
 const char kVModule[]                       = "vmodule";
 
 // Will wait for 60 seconds for a debugger to come to attach to the process.
diff --git a/base/logging.h b/base/logging.h
index c4db4c5..6704325 100644
--- a/base/logging.h
+++ b/base/logging.h
@@ -1,4 +1,4 @@
-// Copyright (c) 2006-2008 The Chromium Authors. All rights reserved.
+// Copyright (c) 2010 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
@@ -81,16 +81,22 @@
 //
 // These always log at the INFO log level (when they log at all).
 // The verbose logging can also be turned on module-by-module.  For instance,
-//    --vmodule=profile=2,icon_loader=1,browser_*=3 --v=0
+//    --vmodule=profile=2,icon_loader=1,browser_*=3,*/chromeos/*=4 --v=0
 // will cause:
 //   a. VLOG(2) and lower messages to be printed from profile.{h,cc}
 //   b. VLOG(1) and lower messages to be printed from icon_loader.{h,cc}
 //   c. VLOG(3) and lower messages to be printed from files prefixed with
 //      "browser"
+//   c. VLOG(4) and lower messages to be printed from files under a
+//     "chromeos" directory.
 //   d. VLOG(0) and lower messages to be printed from elsewhere
 //
 // The wildcarding functionality shown by (c) supports both '*' (match
-// 0 or more characters) and '?' (match any single character) wildcards.
+// 0 or more characters) and '?' (match any single character)
+// wildcards.  Any pattern containing a forward or backward slash will
+// be tested against the whole pathname and not just the module.
+// E.g., "*/foo/bar/*=2" would change the logging level for all code
+// in source files under a "foo/bar" directory.
 //
 // There's also VLOG_IS_ON(n) "verbose level" condition macro. To be used as
 //
diff --git a/base/vlog.cc b/base/vlog.cc
index 6075b0b..3e892d3 100644
--- a/base/vlog.cc
+++ b/base/vlog.cc
@@ -13,11 +13,34 @@ namespace logging {
 
 const int VlogInfo::kDefaultVlogLevel = 0;
 
+VlogInfo::VmodulePattern::VmodulePattern(const std::string& pattern)
+    : pattern(pattern),
+      vlog_level(VlogInfo::kDefaultVlogLevel),
+      match_target(MATCH_MODULE) {
+  // If the pattern contains a {forward,back} slash, we assume that
+  // it's meant to be tested against the entire __FILE__ string.
+  std::string::size_type first_slash = pattern.find_first_of("\\/");
+  if (first_slash != std::string::npos) {
+    // The backslash is an escape character for patterns, so we need
+    // to escape it.  This is okay, because it's highly unlikely that
+    // the user would want to match literal *s or ?s.  However, we may
+    // want to have our own simpler version of MatchPattern() to avoid
+    // these sorts of hacks.
+    ReplaceSubstringsAfterOffset(&this->pattern, first_slash, "\\", "\\\\");
+    match_target = MATCH_FILE;
+  }
+}
+
+VlogInfo::VmodulePattern::VmodulePattern()
+    : vlog_level(VlogInfo::kDefaultVlogLevel),
+      match_target(MATCH_MODULE) {}
+
 VlogInfo::VlogInfo(const std::string& v_switch,
                    const std::string& vmodule_switch)
     : max_vlog_level_(kDefaultVlogLevel) {
   typedef std::pair<std::string, std::string> KVPair;
-  if (!base::StringToInt(v_switch, &max_vlog_level_)) {
+  if (!v_switch.empty() &&
+      !base::StringToInt(v_switch, &max_vlog_level_)) {
     LOG(WARNING) << "Parsed v switch \""
                  << v_switch << "\" as " << max_vlog_level_;
   }
@@ -29,40 +52,49 @@ VlogInfo::VlogInfo(const std::string& v_switch,
   }
   for (std::vector<KVPair>::const_iterator it = kv_pairs.begin();
        it != kv_pairs.end(); ++it) {
-    int vlog_level = kDefaultVlogLevel;
-    if (!base::StringToInt(it->second, &vlog_level)) {
+    VmodulePattern pattern(it->first);
+    if (!base::StringToInt(it->second, &pattern.vlog_level)) {
       LOG(WARNING) << "Parsed vlog level for \""
                    << it->first << "=" << it->second
-                   << "\" as " << vlog_level;
+                   << "\" as " << pattern.vlog_level;
     }
-    vmodule_levels_.push_back(std::make_pair(it->first, vlog_level));
+    vmodule_levels_.push_back(pattern);
   }
 }
 
 VlogInfo::~VlogInfo() {}
 
+namespace {
+
+// Given a path, returns the basename with the extension chopped off
+// (and any -inl suffix).  We avoid using FilePath to minimize the
+// number of dependencies the logging system has.
+base::StringPiece GetModule(const base::StringPiece& file) {
+  base::StringPiece module(file);
+  base::StringPiece::size_type last_slash_pos =
+      module.find_last_of("\\/");
+  if (last_slash_pos != base::StringPiece::npos)
+    module.remove_prefix(last_slash_pos + 1);
+  base::StringPiece::size_type extension_start = module.rfind('.');
+  module = module.substr(0, extension_start);
+  static const char kInlSuffix[] = "-inl";
+  static const int kInlSuffixLen = arraysize(kInlSuffix) - 1;
+  if (module.ends_with(kInlSuffix))
+    module.remove_suffix(kInlSuffixLen);
+  return module;
+}
+
+}  // namespace
+
 int VlogInfo::GetVlogLevel(const base::StringPiece& file) {
   if (!vmodule_levels_.empty()) {
-    base::StringPiece module(file);
-    base::StringPiece::size_type last_slash_pos =
-        module.find_last_of("\\/");
-    if (last_slash_pos != base::StringPiece::npos) {
-      module.remove_prefix(last_slash_pos + 1);
-    }
-    base::StringPiece::size_type extension_start = module.find('.');
-    module = module.substr(0, extension_start);
-    static const char kInlSuffix[] = "-inl";
-    static const int kInlSuffixLen = arraysize(kInlSuffix) - 1;
-    if (module.ends_with(kInlSuffix)) {
-      module.remove_suffix(kInlSuffixLen);
-    }
+    base::StringPiece module(GetModule(file));
     for (std::vector<VmodulePattern>::const_iterator it =
              vmodule_levels_.begin(); it != vmodule_levels_.end(); ++it) {
-      // TODO(akalin): Use a less-heavyweight version of MatchPattern
-      // (we can pretty much assume we're dealing with ASCII).
-      if (MatchPattern(module, it->first)) {
-        return it->second;
-      }
+      base::StringPiece target(
+          (it->match_target == VmodulePattern::MATCH_FILE) ? file : module);
+      if (MatchPattern(target, it->pattern))
+        return it->vlog_level;
     }
   }
   return max_vlog_level_;
diff --git a/base/vlog.h b/base/vlog.h
index d4cffe4..1bdeb24 100644
--- a/base/vlog.h
+++ b/base/vlog.h
@@ -8,7 +8,6 @@
 
 #include <cstddef>
 #include <string>
-#include <utility>
 #include <vector>
 
 #include "base/basictypes.h"
@@ -28,6 +27,11 @@ class VlogInfo {
   // E.g. "my_module=2,foo*=3" would change the logging level for all
   // code in source files "my_module.*" and "foo*.*" ("-inl" suffixes
   // are also disregarded for this matching).
+  //
+  // Any pattern containing a forward or backward slash will be tested
+  // against the whole pathname and not just the module.  E.g.,
+  // "*/foo/bar/*=2" would change the logging level for all code in
+  // source files under a "foo/bar" directory.
   VlogInfo(const std::string& v_switch,
            const std::string& vmodule_switch);
   ~VlogInfo();
@@ -39,7 +43,19 @@ class VlogInfo {
   static const int kDefaultVlogLevel;
 
  private:
-  typedef std::pair<std::string, int> VmodulePattern;
+  // VmodulePattern holds all the information for each pattern parsed
+  // from |vmodule_switch|.
+  struct VmodulePattern {
+    enum MatchTarget { MATCH_MODULE, MATCH_FILE };
+
+    explicit VmodulePattern(const std::string& pattern);
+
+    VmodulePattern();
+
+    std::string pattern;
+    int vlog_level;
+    MatchTarget match_target;
+  };
 
   int max_vlog_level_;
   std::vector<VmodulePattern> vmodule_levels_;
diff --git a/base/vlog_unittest.cc b/base/vlog_unittest.cc
index 95aa1b9a..c69f9cc 100644
--- a/base/vlog_unittest.cc
+++ b/base/vlog_unittest.cc
@@ -26,10 +26,10 @@ TEST_F(VlogTest, NoVmodule) {
   EXPECT_EQ(5, VlogInfo("5", "").GetVlogLevel("test6"));
 }
 
-TEST_F(VlogTest, Vmodule) {
+TEST_F(VlogTest, VmoduleBasic) {
   const char kVSwitch[] = "-1";
   const char kVModuleSwitch[] =
-      "foo=,bar=0,baz=blah,,qux=0blah1,quux=1,corge=5";
+      "foo=,bar=0,baz=blah,,qux=0blah1,quux=1,corge.ext=5";
   VlogInfo vlog_info(kVSwitch, kVModuleSwitch);
   EXPECT_EQ(-1, vlog_info.GetVlogLevel("/path/to/grault.cc"));
   EXPECT_EQ(0, vlog_info.GetVlogLevel("/path/to/foo.cc"));
@@ -38,70 +38,32 @@ TEST_F(VlogTest, Vmodule) {
   EXPECT_EQ(0, vlog_info.GetVlogLevel("baz.h"));
   EXPECT_EQ(0, vlog_info.GetVlogLevel("/another/path/to/qux.h"));
   EXPECT_EQ(1, vlog_info.GetVlogLevel("/path/to/quux"));
-  EXPECT_EQ(5, vlog_info.GetVlogLevel("c:\\path/to/corge.h"));
+  EXPECT_EQ(5, vlog_info.GetVlogLevel("c:\\path/to/corge.ext.h"));
 }
 
-#define BENCHMARK(iters, elapsed, code)                         \
-  do {                                                          \
-    base::TimeTicks start = base::TimeTicks::Now();             \
-    for (int i = 0; i < iters; ++i) code;                       \
-    base::TimeTicks end = base::TimeTicks::Now();               \
-    elapsed = end - start;                                      \
-    double cps = iters / elapsed.InSecondsF();                  \
-    LOG(INFO) << cps << " cps (" << elapsed.InSecondsF()        \
-              << "s elapsed)";                                  \
-  } while (0)
-
-double GetSlowdown(const base::TimeDelta& base,
-                   const base::TimeDelta& elapsed) {
-  return elapsed.InSecondsF() / base.InSecondsF();
-}
-
-
-TEST_F(VlogTest, Perf) {
-  const char* kVlogs[] = {
-    "/path/to/foo.cc",
-    "C:\\path\\to\\bar.h",
-    "/path/to/not-matched.mm",
-    "C:\\path\\to\\baz-inl.mm",
-    "C:\\path\\to\\qux.mm",
-    "/path/to/quux.mm",
-    "/path/to/another-not-matched.mm",
-  };
-  const int kVlogCount = arraysize(kVlogs);
-  const int kBenchmarkIterations = RunningOnValgrind() ? 30000 : 10000000;
-
-  base::TimeDelta null_elapsed;
-  {
-    VlogInfo null_vlog_info("", "");
-    BENCHMARK(kBenchmarkIterations, null_elapsed, {
-      EXPECT_NE(-1, null_vlog_info.GetVlogLevel(kVlogs[i % kVlogCount]));
-    });
-  }
-
-  {
-    VlogInfo small_vlog_info("0", "foo=1,bar=2,baz=3,qux=4,quux=5");
-    base::TimeDelta elapsed;
-    BENCHMARK(kBenchmarkIterations, elapsed, {
-      EXPECT_NE(-1, small_vlog_info.GetVlogLevel(kVlogs[i % kVlogCount]));
-    });
-    LOG(INFO) << "slowdown = " << GetSlowdown(null_elapsed, elapsed)
-              << "x";
-  }
-
-  {
-    VlogInfo pattern_vlog_info("0", "fo*=1,ba?=2,b*?z=3,*ux=4,?uux=5");
-    base::TimeDelta elapsed;
-    BENCHMARK(kBenchmarkIterations, elapsed, {
-      EXPECT_NE(-1, pattern_vlog_info.GetVlogLevel(kVlogs[i % kVlogCount]));
-    });
-    LOG(INFO) << "slowdown = " << GetSlowdown(null_elapsed, elapsed)
-              << "x";
-  }
+TEST_F(VlogTest, VmoduleDirs) {
+  const char kVModuleSwitch[] =
+      "foo/bar.cc=1,baz\\*\\qux.cc=2,*quux/*=3,*/*-inl.h=4";
+  VlogInfo vlog_info("", kVModuleSwitch);
+  EXPECT_EQ(0, vlog_info.GetVlogLevel("/foo/bar.cc"));
+  EXPECT_EQ(0, vlog_info.GetVlogLevel("bar.cc"));
+  EXPECT_EQ(1, vlog_info.GetVlogLevel("foo/bar.cc"));
+
+  EXPECT_EQ(0, vlog_info.GetVlogLevel("baz/grault/qux.h"));
+  EXPECT_EQ(0, vlog_info.GetVlogLevel("/baz/grault/qux.cc"));
+  EXPECT_EQ(0, vlog_info.GetVlogLevel("baz/grault/qux.cc"));
+  EXPECT_EQ(0, vlog_info.GetVlogLevel("baz/grault/blah/qux.cc"));
+  EXPECT_EQ(2, vlog_info.GetVlogLevel("baz\\grault\\qux.cc"));
+  EXPECT_EQ(2, vlog_info.GetVlogLevel("baz\\grault\\blah\\qux.cc"));
+
+  EXPECT_EQ(0, vlog_info.GetVlogLevel("/foo/bar/baz/quux.cc"));
+  EXPECT_EQ(3, vlog_info.GetVlogLevel("/foo/bar/baz/quux/grault.cc"));
+
+  EXPECT_EQ(0, vlog_info.GetVlogLevel("foo/bar/test-inl.cc"));
+  EXPECT_EQ(4, vlog_info.GetVlogLevel("foo/bar/test-inl.h"));
+  EXPECT_EQ(4, vlog_info.GetVlogLevel("foo/bar/baz/blah-inl.h"));
 }
 
-#undef BENCHMARK
-
 }  // namespace
 
 }  // namespace logging