Move SharedMemoryLimits out of WebGraphicsContext3DCommandBufferImpl.

content/browser/renderer_host/compositor_impl_android.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/renderer_host/compositor_impl_android.h"
 
 #include <android/bitmap.h>
 #include <android/native_window_jni.h>
 #include <stdint.h>
 
@@ skipping 129 matching lines @@
   }
 
   bool BindToClient(cc::OutputSurfaceClient* client) override {
     if (!OutputSurface::BindToClient(client))
       return false;
 
     GetCommandBufferProxy()->SetSwapBuffersCompletionCallback(
         swap_buffers_completion_callback_.callback());
 
     populate_gpu_capabilities_callback_.Run(
-        context_provider_->ContextCapabilities().gpu);
+        context_provider_->ContextCapabilities());
     compositor_->AddObserver(this);
 
     client->SetBeginFrameSource(begin_frame_source_.get());
 
     return true;
   }
 
   void DetachFromClient() override {
     client_->SetBeginFrameSource(nullptr);
     OutputSurface::DetachFromClient();
@@ skipping 250 matching lines @@
   if (host_)
     host_->set_has_transparent_background(flag);
 }
 
 void CompositorImpl::SetNeedsComposite() {
   if (!host_->visible())
     return;
   host_->SetNeedsAnimate();
 }
 
-static std::unique_ptr<WebGraphicsContext3DCommandBufferImpl>
-CreateGpuProcessViewContext(
-    const scoped_refptr<gpu::GpuChannelHost>& gpu_channel_host,
-    const gpu::gles2::ContextCreationAttribHelper& attributes,
-    int surface_id) {
-  GURL url("chrome://gpu/Compositor::createContext3D");
-  static const size_t kBytesPerPixel = 4;
-  gfx::DeviceDisplayInfo display_info;
-  size_t full_screen_texture_size_in_bytes =
-      display_info.GetDisplayHeight() *
-      display_info.GetDisplayWidth() *
-      kBytesPerPixel;
-  WebGraphicsContext3DCommandBufferImpl::SharedMemoryLimits limits;
-  limits.command_buffer_size = 64 * 1024;
-  limits.start_transfer_buffer_size = 64 * 1024;
-  limits.min_transfer_buffer_size = 64 * 1024;
-  limits.max_transfer_buffer_size = std::min(
-      3 * full_screen_texture_size_in_bytes, kDefaultMaxTransferBufferSize);
-  limits.mapped_memory_reclaim_limit = 2 * 1024 * 1024;
-  GpuSurfaceTracker* tracker = GpuSurfaceTracker::Get();
-  gpu::SurfaceHandle surface_handle = tracker->GetSurfaceHandle(surface_id);
-  bool share_resources = true;
-  bool automatic_flushes = false;
-  return base::WrapUnique(new WebGraphicsContext3DCommandBufferImpl(
-      surface_handle, url, gpu_channel_host.get(), attributes,
-      gfx::PreferIntegratedGpu, share_resources, automatic_flushes, limits,
-      nullptr));
-}
-
 void CompositorImpl::UpdateLayerTreeHost() {
   client_->UpdateLayerTreeHost();
   if (needs_animate_) {
     needs_animate_ = false;
     root_window_->Animate(base::TimeTicks::Now());
   }
 }
 
 void CompositorImpl::OnGpuChannelEstablished() {
   establish_gpu_channel_timeout_.Stop();
@@ skipping 82 matching lines @@
   DCHECK(window_);
   DCHECK(surface_id_);
 
   BrowserGpuChannelHostFactory* factory =
       BrowserGpuChannelHostFactory::instance();
   // This channel might be lost (and even if it isn't right now, it might
   // still get marked as lost from the IO thread, at any point in time really).
   // But from here on just try and always lead to either
   // DidInitializeOutputSurface() or DidFailToInitializeOutputSurface().
   scoped_refptr<gpu::GpuChannelHost> gpu_channel_host(factory->GetGpuChannel());
+
+  GURL url("chrome://gpu/CompositorImpl::CreateOutputSurface");
+  gpu::SurfaceHandle surface_handle =
+      GpuSurfaceTracker::Get()->GetSurfaceHandle(surface_id_);
+  constexpr bool share_resources = true;
+  constexpr bool automatic_flushes = false;
+
+  // TODO(danakj): Why are these limits lower here? This context is used by the

[danakj, 2016/04/19 02:02:11]

Would like feedback here from sievers/boliu

+  // browser compositor (so limits like RenderWidget make sense), but also by
+  // the display compositor and GpuProcessTransportFactory doesn't set limits
+  // on the display compositor context, and we do issue a fair number of draw
+  // commands?

[piman, 2016/04/19 03:22:17]

We should look into it. In the best case, if I counted right, it takes 172 bytes
in the command buffer for a content quad, and contiguous texture quads (for UI
layers) are coalesced, so only take a bit of transfer buffer + about 150 bytes
of command buffer per batch. It takes more if there's a lot of changes between
quad types. But I think this should account for ~200-300 quads total, or 100-150
quads per frame (assuming max pending = 2). It's not huge, but it's not entirely
unreasonable for Android, I think - we should probably measure to check.

+  gpu::SharedMemoryLimits limits;
+  limits.command_buffer_size = 64 * 1024;
+  limits.start_transfer_buffer_size = 64 * 1024;
+  limits.min_transfer_buffer_size = 64 * 1024;
+  constexpr size_t kBytesPerPixel = 4;
+  const size_t full_screen_texture_size_in_bytes =
+      gfx::DeviceDisplayInfo().GetDisplayHeight() *
+      gfx::DeviceDisplayInfo().GetDisplayWidth() * kBytesPerPixel;
+  limits.max_transfer_buffer_size = std::min(
+      3 * full_screen_texture_size_in_bytes, kDefaultMaxTransferBufferSize);
+  // TODO(danakj): Why this limit on the compositor context?

[danakj, 2016/04/19 02:02:11]

And here

[no sievers, 2016/04/19 20:24:13]

We definitely want to limit all shared memory aggressively on Android in
particular on anything with 1GB or less of RAM. And really it comes down to
having hard limits and avoiding shared memory spikes there that cause us to get
OOM-killed.

But some of the reasoning might be outdated. Today:

for command buffer: like piman said, and I think ericrk@ hooked this up to the
memory tracing so we could see if this is too low.

for transfer buffer (texImage2D/texSubImage2D): I think this would affect
UIResource uploading. which is mostly the omnibox and tab placeholders (the rest
should be more static).

(Note that a mapped memory fast path was also added to TexImage2D so it might
actually use that. It's disabled though if you have 1GB or less of RAM, see
gles2_implementation.cc:max_extra_transfer_buffer_size_.)


for mapped memory (mapTexImage2D, async readPixels): This was put as a hard
limit to avoid spikes. And IIRC would only matter if you started for example a
second readback while one is still in flight. But then again: we only use
texture CopyOutputRequests so the place to limit this is actually the GLHelper
context we allocated in RWHVAndroid and which does the async readback.

[danakj, 2016/04/20 00:01:53]

OK I've turned this one into a TODO, and the above one into a set of comments.
PTAL

[no sievers, 2016/04/20 00:06:29]

sounds/looks good. thanks!

+  limits.mapped_memory_reclaim_limit = 2 * 1024 * 1024;
+
   scoped_refptr<ContextProviderCommandBuffer> context_provider(
       new ContextProviderCommandBuffer(
-          CreateGpuProcessViewContext(gpu_channel_host, attributes,
-                                      surface_id_),
-          DISPLAY_COMPOSITOR_ONSCREEN_CONTEXT));
+          base::WrapUnique(new WebGraphicsContext3DCommandBufferImpl(
+              surface_handle, url, gpu_channel_host.get(), attributes,
+              gfx::PreferIntegratedGpu, share_resources, automatic_flushes,
+              nullptr)),
+          limits, DISPLAY_COMPOSITOR_ONSCREEN_CONTEXT));
   DCHECK(context_provider.get());
 
   std::unique_ptr<cc::OutputSurface> real_output_surface(
       new OutputSurfaceWithoutParent(
           this, context_provider,
           base::Bind(&CompositorImpl::PopulateGpuCapabilities,
                      base::Unretained(this)),
           base::WrapUnique(new ExternalBeginFrameSource(this))));
 
   cc::SurfaceManager* manager = GetSurfaceManager();
@@ skipping 89 matching lines @@
 
 void CompositorImpl::SetNeedsAnimate() {
   needs_animate_ = true;
   if (!host_->visible())
     return;
 
   host_->SetNeedsAnimate();
 }
 
 }  // namespace content