Improve GPU software rendering list histograms.

content/browser/gpu/gpu_data_manager.cc

 // Copyright (c) 2011 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.
 
 #include "content/browser/gpu/gpu_data_manager.h"
 
 #if defined(OS_MACOSX)
 #include <CoreGraphics/CGDisplayConfiguration.h>
 #endif
 
 #include "base/command_line.h"
 #include "base/metrics/histogram.h"
 #include "base/stringprintf.h"
 #include "base/string_number_conversions.h"
+#include "base/sys_info.h"
+#include "base/values.h"
+#include "base/version.h"
 #include "content/browser/browser_thread.h"
 #include "content/browser/gpu/gpu_blacklist.h"
 #include "content/browser/gpu/gpu_process_host.h"
 #include "content/common/content_client.h"
 #include "content/common/content_switches.h"
 #include "content/common/gpu/gpu_messages.h"
 #include "content/gpu/gpu_info_collector.h"
 #include "ui/gfx/gl/gl_implementation.h"
 #include "ui/gfx/gl/gl_switches.h"
 
@@ skipping 47 matching lines @@
       it != node.children.end();
       ++it) {
     ListValue* sublist = DxDiagNodeToList(it->second);
     list->Append(NewDescriptionValuePair(it->first, sublist));
   }
   return list;
 }
 
 #endif  // OS_WIN
 
+std::string GetOSString() {
+  std::string rt;
+#if defined(OS_CHROMEOS)
+  rt = "ChromeOS";
+#elif defined(OS_WIN)
+  rt = "Win";
+  std::string version_str = base::SysInfo::OperatingSystemVersion();
+  size_t pos = version_str.find_first_not_of("0123456789.");
+  if (pos != std::string::npos)
+    version_str = version_str.substr(0, pos);
+  scoped_ptr<Version> os_version(Version::GetVersionFromString(version_str));
+  DCHECK(os_version.get());
+  const std::vector<uint16>& version_numbers = os_version->components();
+  DCHECK_GE(version_numbers.size(), 2);

[vangelis, 2011/08/17 17:40:17]

I think that in addition to the DCHECK we'll need to bail out safely if for some
reason we don't get enough fields in the version (to avoid nasty crashes).

[Zhenyao Mo, 2011/08/17 19:29:54]

Done.

+  if (version_numbers[0] == 5)
+    rt += "XP";
+  else if (version_numbers[0] == 6 && version_numbers[1] == 0)
+    rt += "Vista";
+  else if (version_numbers[0] == 6 && version_numbers[1] == 1)
+    rt += "7";
+#elif defined(OS_LINUX)
+  rt = "Linux";
+#elif defined(OS_MACOSX)
+  rt = "Mac";
+#else
+  rt = "UnknownOS";
+#endif
+  return rt;
+}
+
+void LogToHistogram(
+    const std::string& name, uint32 sample, uint32 boundary_value) {
+  base::LinearHistogram::FactoryGet(name, 1, boundary_value, boundary_value + 1,
+      base::Histogram::kUmaTargetedHistogramFlag)->Add(sample);
+}
+
 }  // namespace anonymous
 
 GpuDataManager::GpuDataManager()
     : complete_gpu_info_already_requested_(false) {
   // Certain tests doesn't go through the browser startup path that
   // initializes GpuDataManager on FILE thread; therefore, it is initialized
   // on UI thread later, and we skip the preliminary gpu info collection
   // in such situation.
   if (!BrowserThread::CurrentlyOn(BrowserThread::FILE))
     return;
@@ skipping 262 matching lines @@
     gpu_feature_flags_.set_flags(0);
     return;
   }
 
   {
     base::AutoLock auto_lock(gpu_info_lock_);
     gpu_feature_flags_ = gpu_blacklist->DetermineGpuFeatureFlags(
         GpuBlacklist::kOsAny, NULL, gpu_info_);
   }
 
-  uint32 max_entry_id = gpu_blacklist->max_entry_id();
-  if (!gpu_feature_flags_.flags()) {
-    UMA_HISTOGRAM_ENUMERATION("GPU.BlacklistTestResultsPerEntry",
-        0, max_entry_id + 1);
-    return;
-  }
-
   // Notify clients that GpuInfo state has changed
   RunGpuInfoUpdateCallbacks();
 
-  // TODO(zmo): move histograming to GpuBlacklist::DetermineGpuFeatureFlags.
-  std::vector<uint32> flag_entries;
-  gpu_blacklist->GetGpuFeatureFlagEntries(
-      GpuFeatureFlags::kGpuFeatureAll, flag_entries);
-  DCHECK_GT(flag_entries.size(), 0u);
-  for (size_t i = 0; i < flag_entries.size(); ++i) {
-    UMA_HISTOGRAM_ENUMERATION("GPU.BlacklistTestResultsPerEntry",
-        flag_entries[i], max_entry_id + 1);
+  uint32 max_entry_id = gpu_blacklist->max_entry_id();
+
+  const GpuFeatureFlags::GpuFeatureType kGpuFeatures[] = {
+      GpuFeatureFlags::kGpuFeatureAccelerated2dCanvas,
+      GpuFeatureFlags::kGpuFeatureAcceleratedCompositing,
+      GpuFeatureFlags::kGpuFeatureWebgl
+  };
+  const std::string kGpuBlacklistHistogramPerEntryNames[] = {
+      "GPU.BlacklistAccelerated2dCanvasTestResultsPerEntry",
+      "GPU.BlacklistAcceleratedCompositingTestResultsPerEntry",
+      "GPU.BlacklistWebglTestResultsPerEntry"
+  };
+  const std::string kOSString = GetOSString();
+  const std::string kGpuBlacklistHistogramNames[] = {
+      "GPU.BlacklistAccelerated2dCanvasTestResults" + kOSString,
+      "GPU.BlacklistAcceleratedCompositingTestResults" + kOSString,
+      "GPU.BlacklistWebglTestResults" + kOSString
+  };
+
+  uint32 flags = gpu_feature_flags_.flags();
+
+  for (size_t i = 0;
+       i < sizeof(kGpuFeatures) / sizeof(GpuFeatureFlags::GpuFeatureType);
+       ++i) {
+    if ((flags & kGpuFeatures[i]) == 0) {
+      LogToHistogram(kGpuBlacklistHistogramPerEntryNames[i],
+          0, max_entry_id + 1);
+      LogToHistogram(kGpuBlacklistHistogramNames[i], 0, 2);

[vangelis, 2011/08/17 17:40:17]

Sorry, I should have spotted that earlier but I don't think we want to be adding
a tick to the kGpuBlacklistHistograPerEntry for every feature that's disabled. 
I think a better strategy would be to update that histogram if any of the
features is blacklisted.
 

[Zhenyao Mo, 2011/08/17 19:29:54]

Done.

+    } else {
+      std::vector<uint32> flag_entries;
+      gpu_blacklist->GetGpuFeatureFlagEntries(kGpuFeatures[i], flag_entries);
+      DCHECK_GT(flag_entries.size(), 0u);
+      for (size_t j = 0; j < flag_entries.size(); ++j) {
+        LogToHistogram(kGpuBlacklistHistogramPerEntryNames[i],
+            flag_entries[j], max_entry_id + 1);
+      }
+      LogToHistogram(kGpuBlacklistHistogramNames[i], 1, 2);
+    }
   }
 }
 
 GpuBlacklist* GpuDataManager::GetGpuBlacklist() {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
   const CommandLine& browser_command_line = *CommandLine::ForCurrentProcess();
   if (browser_command_line.HasSwitch(switches::kIgnoreGpuBlacklist) ||
       browser_command_line.GetSwitchValueASCII(
           switches::kUseGL) == gfx::kGLImplementationOSMesaName)
     return NULL;
   // No need to return an empty blacklist.
   if (gpu_blacklist_.get() != NULL && gpu_blacklist_->max_entry_id() == 0)
     return NULL;
   return gpu_blacklist_.get();
 }