Re-land: content: Use NumberOfProcessors() / 2 raster threads.

content/browser/gpu/compositor_util.cc

 // Copyright (c) 2012 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/compositor_util.h"
 
 #include "base/command_line.h"
 #include "base/logging.h"
 #include "base/metrics/field_trial.h"
 #include "base/strings/string_number_conversions.h"
 #include "base/sys_info.h"
 #include "build/build_config.h"
+#include "cc/base/math_util.h"
 #include "cc/base/switches.h"
 #include "content/browser/gpu/gpu_data_manager_impl.h"
 #include "content/public/common/content_switches.h"
 #include "gpu/config/gpu_feature_type.h"
 
 namespace content {
 
 namespace {
 
 static bool IsGpuRasterizationBlacklisted() {
   GpuDataManagerImpl* manager = GpuDataManagerImpl::GetInstance();
   return manager->IsFeatureBlacklisted(
         gpu::GPU_FEATURE_TYPE_GPU_RASTERIZATION);
 }
 
 const char* kGpuCompositingFeatureName = "gpu_compositing";
 const char* kWebGLFeatureName = "webgl";
 const char* kRasterizationFeatureName = "rasterization";
 const char* kThreadedRasterizationFeatureName = "threaded_rasterization";
 const char* kMultipleRasterThreadsFeatureName = "multiple_raster_threads";
 
 const int kMinRasterThreads = 1;
 const int kMaxRasterThreads = 64;

[danakj, 2015/05/13 20:01:23]

does 64 threads really ever make sense? at what point does a tile per thread get
ridiculous?

[reveman, 2015/05/13 20:24:35]

We're going to use these threads for more than raster in the near future (e.g.
video plane copy operations, fav icon decode, etc.) so it's better to think of
these as child process worker threads. Yes, the command line switch and these
constants needs to be renamed at some point.

I'm happy to lower it though, just don't know to what.

 
 const int kMinMSAASampleCount = 0;
 
 struct GpuFeatureInfo {
   std::string name;
   bool blocked;
   bool disabled;
   std::string disabled_description;
   bool fallback_to_software;
 };
@@ skipping 157 matching lines @@
 
 bool IsImplSidePaintingEnabled() {
   const base::CommandLine& command_line =
       *base::CommandLine::ForCurrentProcess();
   if (command_line.HasSwitch(switches::kDisableImplSidePainting))
     return false;
   return true;
 }
 
 int NumberOfRendererRasterThreads() {
-  int num_raster_threads = 1;
+  int num_raster_threads = cc::MathUtil::ClampToRange(
+      base::SysInfo::NumberOfProcessors() / 2,
+      kMinRasterThreads, kMaxRasterThreads);

[danakj, 2015/05/13 20:01:23]

nit: how about do the computation for a number here and below without clamping,
then clamp at the end once.

[reveman, 2015/05/13 20:24:35]

I did that first but didn't like it as it's already clamped when
force_num_raster_threads is true. I guess I can refactor
ForceNumberOfRendererRasterThreads a bit to make more sense.

 
-  // Async uploads uses its own thread, so allow an extra thread when async
-  // uploads is not in use.
-  bool allow_extra_thread =
+  // Async uploads is used when neither zero-copy nor one-copy is enabled and
+  // it uses its own thread, so reduce the number of raster threads when async
+  // uploads is in use.
+  bool async_uploads_is_used =
       IsZeroCopyUploadEnabled() || IsOneCopyUploadEnabled();
-  if (base::SysInfo::NumberOfProcessors() >= 4 && allow_extra_thread)
-    num_raster_threads = 2;
+  if (async_uploads_is_used)
+    num_raster_threads = std::max(kMinRasterThreads, num_raster_threads - 1);
 
   int force_num_raster_threads = ForceNumberOfRendererRasterThreads();
   if (force_num_raster_threads)
     num_raster_threads = force_num_raster_threads;
 
   return num_raster_threads;
 }
 
 bool IsOneCopyUploadEnabled() {
   if (IsZeroCopyUploadEnabled())
@@ skipping 210 matching lines @@
     }
   }
   return problem_list;
 }
 
 std::vector<std::string> GetDriverBugWorkarounds() {
   return GpuDataManagerImpl::GetInstance()->GetDriverBugWorkarounds();
 }
 
 }  // namespace content