Plumbing explicit share groups through context creation

content/common/gpu/client/webgraphicscontext3d_command_buffer_impl.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/common/gpu/client/webgraphicscontext3d_command_buffer_impl.h"
 
 #include "third_party/khronos/GLES2/gl2.h"
 #ifndef GL_GLEXT_PROTOTYPES
 #define GL_GLEXT_PROTOTYPES 1
 #endif
 #include "third_party/khronos/GLES2/gl2ext.h"
 
 #include <algorithm>
 #include <map>
 
 #include "base/atomicops.h"
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/debug/trace_event.h"
 #include "base/lazy_instance.h"
 #include "base/logging.h"
 #include "base/message_loop/message_loop.h"
 #include "base/metrics/field_trial.h"
 #include "base/metrics/histogram.h"
-#include "base/synchronization/lock.h"
 #include "content/common/gpu/client/gpu_channel_host.h"
 #include "content/public/common/content_constants.h"
 #include "content/public/common/content_switches.h"
 #include "gpu/GLES2/gl2extchromium.h"
 #include "gpu/command_buffer/client/gles2_cmd_helper.h"
 #include "gpu/command_buffer/client/gles2_implementation.h"
 #include "gpu/command_buffer/client/gles2_lib.h"
 #include "gpu/command_buffer/client/gles2_trace_implementation.h"
 #include "gpu/command_buffer/client/transfer_buffer.h"
 #include "gpu/command_buffer/common/constants.h"
 #include "gpu/command_buffer/common/gpu_memory_allocation.h"
 #include "gpu/command_buffer/common/mailbox.h"
 #include "gpu/skia_bindings/gl_bindings_skia_cmd_buffer.h"
 #include "third_party/skia/include/core/SkTypes.h"
 
 namespace content {
 
 namespace {
 
 static base::LazyInstance<base::Lock>::Leaky
-    g_all_shared_contexts_lock = LAZY_INSTANCE_INITIALIZER;
+    g_default_share_groups_lock = LAZY_INSTANCE_INITIALIZER;
 
-typedef std::multimap<GpuChannelHost*, WebGraphicsContext3DCommandBufferImpl*>
-    ContextMap;
-static base::LazyInstance<ContextMap> g_all_shared_contexts =
+typedef std::map<GpuChannelHost*,
+    scoped_refptr<WebGraphicsContext3DCommandBufferImpl::ShareGroup> >
+    ShareGroupMap;
+static base::LazyInstance<ShareGroupMap> g_default_share_groups =
     LAZY_INSTANCE_INITIALIZER;
 
+WebGraphicsContext3DCommandBufferImpl::ShareGroup* defaultShareGroupForHost(

[piman, 2014/01/08 01:39:24]

nit: chrome style - DefaultShareGroupForHost. Maybe GetDefaultShareGroupForHost
?

[Ken Russell (switch to Gerrit), 2014/01/08 02:40:39]

Since a lock is needed for the share group map, it seems risky and racy to be
returning a raw pointer to the ShareGroup from this function. Since the
WebGraphicsContext3DCommandBufferImpl ref-counts the ShareGroup, it seems
plausible the WGC3D might go away and the ShareGroup be deleted between the time
this function returns and the time the caller ref-counts the ShareGroup. I think
this can be solved by returning a scoped_refptr<WGC3DCBI::ShareGroup*>.

[no sievers, 2014/01/08 14:03:54]

Yes, good catch. For example:
1) WGC3D is constructed on impl thread and gets raw pointer to default share
group, but does not hold a reference yet
2) OnGpuChannelLost() is called on main thread for another context (in the
group) which clears g_default_share_groups
3) The context from 2) is deleted on the main thread which releases the last
reference to ShareGroup causing it to get deleted

[bajones, 2014/01/08 21:24:37]

Good catch. Fixed.

+    GpuChannelHost* host) {
+  base::AutoLock lock(g_default_share_groups_lock.Get());
+
+  ShareGroupMap& share_groups = g_default_share_groups.Get();
+  ShareGroupMap::iterator it = share_groups.find(host);
+  if (it == share_groups.end()) {
+    scoped_refptr<WebGraphicsContext3DCommandBufferImpl::ShareGroup> group =
+        new WebGraphicsContext3DCommandBufferImpl::ShareGroup();
+    share_groups[host] = group;
+    return group;
+  }
+  return it->second;
+}
+
 uint32_t GenFlushID() {
   static base::subtle::Atomic32 flush_id = 0;
 
   base::subtle::Atomic32 my_id = base::subtle::Barrier_AtomicIncrement(
       &flush_id, 1);
   return static_cast<uint32_t>(my_id);
 }
 
 // Singleton used to initialize and terminate the gles2 library.
 class GLES2Initializer {
@@ skipping 132 matching lines @@
   graphics_context_->OnErrorMessage(msg, id);
 }
 
 WebGraphicsContext3DCommandBufferImpl::SharedMemoryLimits::SharedMemoryLimits()
     : command_buffer_size(kDefaultCommandBufferSize),
       start_transfer_buffer_size(kDefaultStartTransferBufferSize),
       min_transfer_buffer_size(kDefaultMinTransferBufferSize),
       max_transfer_buffer_size(kDefaultMaxTransferBufferSize),
       mapped_memory_reclaim_limit(gpu::gles2::GLES2Implementation::kNoLimit) {}
 
+WebGraphicsContext3DCommandBufferImpl::ShareGroup::ShareGroup() {
+}
+
+WebGraphicsContext3DCommandBufferImpl::ShareGroup::~ShareGroup() {
+  DCHECK(contexts_.empty());
+}
+
 WebGraphicsContext3DCommandBufferImpl::WebGraphicsContext3DCommandBufferImpl(
     int surface_id,
     const GURL& active_url,
     GpuChannelHost* host,
     const Attributes& attributes,
     bool bind_generates_resources,
-    const SharedMemoryLimits& limits)
+    const SharedMemoryLimits& limits,
+    WebGraphicsContext3DCommandBufferImpl* share_context)
     : initialize_failed_(false),
       visible_(false),
       host_(host),
       surface_id_(surface_id),
       active_url_(active_url),
       context_lost_callback_(0),
       context_lost_reason_(GL_NO_ERROR),
       error_message_callback_(0),
       attributes_(attributes),
       gpu_preference_(attributes.preferDiscreteGPU ? gfx::PreferDiscreteGpu
                                                    : gfx::PreferIntegratedGpu),
       weak_ptr_factory_(this),
       initialized_(false),
       gl_(NULL),
       bind_generates_resources_(bind_generates_resources),
       mem_limits_(limits),
       flush_id_(0) {
+  if (share_context) {

[no sievers, 2014/01/08 14:03:54]

DCHECK(!attributes_.shareResources);

On the other hand if |share_context| was created with shareResources, that seems
confusing but looks like it would still work as expected, right? (Identical to
creating this context with shareResources == true).

[bajones, 2014/01/08 21:24:37]

Yes, that's correct. Slightly confusing, but I think it's well behaved as far as
these things go. (Accidentally sharing with more contexts than intended is
probably better than accidentally sharing with less than expected.)

[no sievers, 2014/01/09 13:39:42]

Sounds good. I'd still add the DCHECK(!attributes_.shareResources) though since
it's different (compared to share_context->shareResources == true) and it would
be illegal to request to be in the global share group and a specific other one
at the same time.

+    share_group_ = share_context->share_group_;

[no sievers, 2014/01/08 14:03:54]

It's possible that host_ != share_context->host_ if the context was lost.
Should we make sure makeCurrent() later will fail consistently in that case by
setting host_ to NULL here?
Or should it create a new share group here if share_context's channel was lost?

With the default share group the recovery is easier here, because it doesn't
matter which context gets recreated first and all new contexts will end up in
the same new share group once the channel was reestablished.

If you explicitly pass in the share group (which might be referencing a lost
channel) it's maybe a bit trickier, because if you create context B by passing
in A, you have to recreate A first.

So it also depends on what the logic for this will be like in Blink.

[bajones, 2014/01/08 21:24:37]

Hm... thanks for pointing this out. I need to think about it a bit further.

[no sievers, 2014/01/09 13:39:42]

Can we just do this maybe to mimick the behavior of the default share group:
- passing in a share_context whose channel is lost will cause this context to be
created with a fresh ShareGroup
- in Blink we always track the latest context that was created to use as a
share_context

When the channel is lost, we know that all contexts will eventually be recreated
and therefore all will end up using the same new ShareGroup at that point.

+  } else {
+    share_group_ = attributes_.shareResources ? defaultShareGroupForHost(host)
+                                              : new ShareGroup();
+  }
 }
 
 WebGraphicsContext3DCommandBufferImpl::
     ~WebGraphicsContext3DCommandBufferImpl() {
   if (real_gl_) {
     real_gl_->SetErrorMessageCallback(NULL);
   }
 
   Destroy();
 }
@@ skipping 49 matching lines @@
     attributes_.stencil = pvalues[2] > 0;
     attributes_.antialias = pvalues[3] > 0;
   }
 
   visible_ = true;
   initialized_ = true;
   return true;
 }
 
 bool WebGraphicsContext3DCommandBufferImpl::InitializeCommandBuffer(
-    bool onscreen) {
+    bool onscreen, WebGraphicsContext3DCommandBufferImpl* share_context) {
   if (!host_.get())
     return false;
-  // We need to lock g_all_shared_contexts to ensure that the context we picked
-  // for our share group isn't deleted.
-  // (There's also a lock in our destructor.)
-  base::AutoLock lock(g_all_shared_contexts_lock.Get());
-  CommandBufferProxyImpl* share_group = NULL;
-  if (attributes_.shareResources) {
-    ContextMap& all_contexts = g_all_shared_contexts.Get();
-    ContextMap::const_iterator it = all_contexts.find(host_.get());
-    if (it != all_contexts.end())
-      share_group = it->second->command_buffer_.get();
+
+  CommandBufferProxyImpl* share_group_command_buffer = NULL;
+
+  if (share_context) {
+    share_group_command_buffer = share_context->command_buffer_.get();
   }
 
   std::vector<int32> attribs;
   attribs.push_back(ALPHA_SIZE);
   attribs.push_back(attributes_.alpha ? 8 : 0);
   attribs.push_back(DEPTH_SIZE);
   attribs.push_back(attributes_.depth ? 24 : 0);
   attribs.push_back(STENCIL_SIZE);
   attribs.push_back(attributes_.stencil ? 8 : 0);
   attribs.push_back(SAMPLES);
   attribs.push_back(attributes_.antialias ? 4 : 0);
   attribs.push_back(SAMPLE_BUFFERS);
   attribs.push_back(attributes_.antialias ? 1 : 0);
   attribs.push_back(FAIL_IF_MAJOR_PERF_CAVEAT);
   attribs.push_back(attributes_.failIfMajorPerformanceCaveat ? 1 : 0);
   attribs.push_back(NONE);
 
   // Create a proxy to a command buffer in the GPU process.
   if (onscreen) {
     command_buffer_.reset(host_->CreateViewCommandBuffer(
         surface_id_,
-        share_group,
+        share_group_command_buffer,
         attribs,
         active_url_,
         gpu_preference_));
   } else {
     command_buffer_.reset(host_->CreateOffscreenCommandBuffer(
         gfx::Size(1, 1),
-        share_group,
+        share_group_command_buffer,
         attribs,
         active_url_,
         gpu_preference_));
   }
 
   if (!command_buffer_)
     return false;
 
   // Initialize the command buffer.
   return command_buffer_->Initialize();
 }
 
-bool WebGraphicsContext3DCommandBufferImpl::CreateContext(
-    bool onscreen) {
+bool WebGraphicsContext3DCommandBufferImpl::CreateContext(bool onscreen) {
   // Ensure the gles2 library is initialized first in a thread safe way.
   g_gles2_initializer.Get();
 
-  if (!command_buffer_ &&
-      !InitializeCommandBuffer(onscreen)) {
-    return false;
+  scoped_refptr<gpu::gles2::ShareGroup> gles2_share_group;
+
+  scoped_ptr<base::AutoLock> share_group_lock;
+  if (!command_buffer_) {
+    WebGraphicsContext3DCommandBufferImpl* share_context = NULL;
+
+    share_group_lock.reset(new base::AutoLock(share_group_->lock()));
+    share_context = share_group_->GetAnyContextLocked();
+
+    if (!InitializeCommandBuffer(onscreen, share_context))
+      return false;
+
+    if (share_context)
+      gles2_share_group = share_context->GetImplementation()->share_group();

[no sievers, 2014/01/08 14:03:54]

Can you keep the DCHECK, i.e. DCHECK(!share_context || gles2_share_group.get())?

+
+    share_group_->AddContextLocked(this);
   }
 
   // Create the GLES2 helper, which writes the command buffer protocol.
   gles2_helper_.reset(new gpu::gles2::GLES2CmdHelper(command_buffer_.get()));
   if (!gles2_helper_->Initialize(mem_limits_.command_buffer_size))
     return false;
 
   if (attributes_.noAutomaticFlushes)
     gles2_helper_->SetAutomaticFlushes(false);
 
   // Create a transfer buffer used to copy resources between the renderer
   // process and the GPU process.
   transfer_buffer_ .reset(new gpu::TransferBuffer(gles2_helper_.get()));
 
   DCHECK(host_.get());
-  scoped_ptr<base::AutoLock> lock;
-  scoped_refptr<gpu::gles2::ShareGroup> share_group;
-  if (attributes_.shareResources) {
-    // Make sure two clients don't try to create a new ShareGroup
-    // simultaneously.
-    lock.reset(new base::AutoLock(g_all_shared_contexts_lock.Get()));
-    ContextMap& all_contexts = g_all_shared_contexts.Get();
-    ContextMap::const_iterator it = all_contexts.find(host_.get());
-    if (it != all_contexts.end()) {
-      share_group = it->second->GetImplementation()->share_group();
-      DCHECK(share_group);
-    }
-  }
 
   const CommandLine& command_line = *CommandLine::ForCurrentProcess();
   bool free_command_buffer_when_invisible =
       command_line.HasSwitch(switches::kEnablePruneGpuCommandBuffers);
 
   // Create the object exposing the OpenGL API.
   real_gl_.reset(new gpu::gles2::GLES2Implementation(
       gles2_helper_.get(),
-      share_group,
+      gles2_share_group,
       transfer_buffer_.get(),
       bind_generates_resources_,
       free_command_buffer_when_invisible,
       command_buffer_.get()));
   gl_ = real_gl_.get();
 

[no sievers, 2014/01/08 14:03:54]

Can we still release the lock here so it isn't held while we initialize the
gles2 impl like before?

-  if (attributes_.shareResources) {
-    // Don't add ourselves to the list before others can get to our ShareGroup.
-    g_all_shared_contexts.Get().insert(std::make_pair(host_.get(), this));
-    lock.reset();
-  }
-
   if (!real_gl_->Initialize(
       mem_limits_.start_transfer_buffer_size,
       mem_limits_.min_transfer_buffer_size,
       mem_limits_.max_transfer_buffer_size,
       mem_limits_.mapped_memory_reclaim_limit)) {
     return false;
   }
 
   if (CommandLine::ForCurrentProcess()->HasSwitch(
       switches::kEnableGpuClientTracing)) {
@@ skipping 14 matching lines @@
   return true;
 }
 
 uint32_t WebGraphicsContext3DCommandBufferImpl::lastFlushID() {
   return flush_id_;
 }
 
 DELEGATE_TO_GL_R(insertSyncPoint, InsertSyncPointCHROMIUM, unsigned int)
 
 void WebGraphicsContext3DCommandBufferImpl::Destroy() {
-  if (host_.get()) {
-    base::AutoLock lock(g_all_shared_contexts_lock.Get());
-    ContextMap& all_contexts = g_all_shared_contexts.Get();
-    ContextMap::iterator it = std::find(
-        all_contexts.begin(),
-        all_contexts.end(),
-        std::pair<GpuChannelHost* const,
-                  WebGraphicsContext3DCommandBufferImpl*>(host_.get(), this));
-    if (it != all_contexts.end())
-      all_contexts.erase(it);
-  }
+  share_group_->RemoveContext(this);
 
   if (gl_) {
     // First flush the context to ensure that any pending frees of resources
     // are completed. Otherwise, if this context is part of a share group,
     // those resources might leak. Also, any remaining side effects of commands
     // issued on this context might not be visible to other contexts in the
     // share group.
     gl_->Flush();
     gl_ = NULL;
   }
@@ skipping 711 matching lines @@
   gpu::CommandBuffer::State state = command_buffer_->GetLastState();
   return state.error == gpu::error::kLostContext;
 }
 
 // static
 WebGraphicsContext3DCommandBufferImpl*
 WebGraphicsContext3DCommandBufferImpl::CreateOffscreenContext(
     GpuChannelHost* host,
     const WebGraphicsContext3D::Attributes& attributes,
     const GURL& active_url,
-    const SharedMemoryLimits& limits) {
+    const SharedMemoryLimits& limits,
+    WebGraphicsContext3DCommandBufferImpl* share_context) {
   if (!host)
     return NULL;
+
   return new WebGraphicsContext3DCommandBufferImpl(0,
                                                    active_url,
                                                    host,
                                                    attributes,
                                                    false,
-                                                   limits);
+                                                   limits,
+                                                   share_context);
 }
 
 DELEGATE_TO_GL_5(texImageIOSurface2DCHROMIUM, TexImageIOSurface2DCHROMIUM,
                  WGC3Denum, WGC3Dint, WGC3Dint, WGC3Duint, WGC3Duint)
 
 DELEGATE_TO_GL_5(texStorage2DEXT, TexStorage2DEXT,
                  WGC3Denum, WGC3Dint, WGC3Duint, WGC3Dint, WGC3Dint)
 
 WebGLId WebGraphicsContext3DCommandBufferImpl::createQueryEXT() {
   GLuint o;
@@ skipping 152 matching lines @@
 
 }  // anonymous namespace
 
 void WebGraphicsContext3DCommandBufferImpl::OnGpuChannelLost() {
   context_lost_reason_ = convertReason(
       command_buffer_->GetLastState().context_lost_reason);
   if (context_lost_callback_) {
     context_lost_callback_->onContextLost();
   }
 
+  share_group_->RemoveAllContexts();
+
   DCHECK(host_.get());
   {
-    base::AutoLock lock(g_all_shared_contexts_lock.Get());
-    g_all_shared_contexts.Get().erase(host_.get());
+    base::AutoLock lock(g_default_share_groups_lock.Get());
+    g_default_share_groups.Get().erase(host_.get());
   }
 }
 
 void WebGraphicsContext3DCommandBufferImpl::OnErrorMessage(
     const std::string& message, int id) {
   if (error_message_callback_) {
     blink::WebString str = blink::WebString::fromUTF8(message.c_str());
     error_message_callback_->onErrorMessage(str, id);
   }
 }
 
 }  // namespace content