Service/ClientDiscardableManager

gpu/command_buffer/service/service_discardable_manager.cc

Index: gpu/command_buffer/service/service_discardable_manager.cc
diff --git a/gpu/command_buffer/service/service_discardable_manager.cc b/gpu/command_buffer/service/service_discardable_manager.cc
new file mode 100644
index 0000000000000000000000000000000000000000..6a0eb2f5012ac5e389092c0d4268cf6692925096
--- /dev/null
+++ b/gpu/command_buffer/service/service_discardable_manager.cc
@@ -0,0 +1,82 @@
+// Copyright (c) 2017 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 "gpu/command_buffer/service/service_discardable_manager.h"
+
+#include "base/memory/singleton.h"
+#include "gpu/command_buffer/service/texture_manager.h"
+
+namespace gpu {
+namespace {
+// TODO(ericrk): Arbitrary limit, refine this once we actually use this class in
+// production. crbug.com/706456
+const size_t kMaxSize = 256 * 1024 * 1024;
+}
+
+ServiceDiscardableManager::ServiceDiscardableManager()
+    : entries_(EntryCache::NO_AUTO_EVICT) {}
+ServiceDiscardableManager::~ServiceDiscardableManager() = default;
+
+void ServiceDiscardableManager::InsertLockedTexture(
+    uint32_t texture_id,
+    size_t texture_size,
+    gles2::ContextGroup* context_group,
+    ServiceDiscardableHandle handle) {
+  total_size_ += texture_size;
+  entries_.Put({texture_id, context_group},
+               GpuDiscardableEntry{handle, texture_size});
+  EnforceLimits();
+}
+
+bool ServiceDiscardableManager::UnlockTexture(
+    uint32_t texture_id,
+    const gles2::ContextGroup* context_group) {
+  auto found = entries_.Get({texture_id, context_group});
+  if (found == entries_.end())
+    return false;
+
+  found->second.handle.Unlock();
+  EnforceLimits();
+  return true;
+}
+
+void ServiceDiscardableManager::OnTextureDeleted(
+    uint32_t texture_id,
+    const gles2::ContextGroup* context_group) {

[piman, 2017/05/02 22:21:03]

You probably also need a call when the whole context group goes away - we delete
the textures then, but we don't go through the
GLES2DecoderImpl::DeleteTexturesHelper path for that (since it's not the result
of DeleteTextures commands). See follow-up CL.
Alternatively, you could have the TextureManager issue this (maybe from
~TextureRef, or TextureManager::StopTracking), so that it's called consistently
in all the paths that delete client textures.

[ericrk, 2017/05/10 21:36:03]

Updated to handle this in TextureManager.

+  auto found = entries_.Get({texture_id, context_group});
+  if (found == entries_.end())
+    return;
+
+  found->second.handle.ForceDelete();
+  total_size_ -= found->second.size;
+  entries_.Erase(found);
+}
+
+void ServiceDiscardableManager::EnforceLimits() {
+  for (auto it = entries_.rbegin(); it != entries_.rend();) {
+    if (total_size_ <= kMaxSize) {
+      return;
+    }
+    if (!it->second.handle.Delete()) {
+      ++it;
+      continue;
+    }
+
+    total_size_ -= it->second.size;
+    it->first.context_group->texture_manager()->RemoveTexture(
+        it->first.texture_id);

[piman, 2017/05/02 22:21:03]

I think that for consistency, we should do something a little different wrt
texture id management - that becomes more apparent with the follow-up CL.
Basically the question is what should happen to the (client) texture ids while
the texture is unlocked. With this, whether a GL call that references (possibly
indirectly) an unlocked texture succeeds, fails, or silently does something
inconsistent, is not something the client can reason about (because unlocked
textures can be deleted at any time).

So, for example:
glUnlockDiscardableTextureCHROMIUM(texture_id);
[...]  // texture may get evicted
glBindTexture(GL_TEXTURE_2D, texture_id);

would either succeed or fail depending on whether the texture was evicted.


Probably worse is when >1 context, something like
Context 1:
glUnlockDiscardableTextureCHROMIUM(texture_id);
[...]  // texture may get evicted
glDeleteTextures(1, &texture_id);

Context 2, on other thread:
glBindTexture(GL_TEXTURE_2D, texture_id);

which will probably cause inconsistencies between client and service depending
on the order of execution (texture gets evicted, the BindTexture may implicitly
recreate the texture, which then gets deleted by the DeleteTextures).


It would be preferable to deterministically have glBindTexture (and other
operations on the texture) always fail (if you want to bind the texture, you
have to lock it), as well as making sure HandleUnlockDiscardableTextureCHROMIUM
does the same work as what DeleteTexturesHelper would have done if we had
deleted instead of unlocked, in terms of resetting the bind points that
reference that texture id.


I think the right way to look at it is, at unlock time, to keep a reference to
the texture (basically, a TextureRef) in the ServiceDiscardableManager, but
actually remove it from the TextureManager. Basically, the
ServiceDiscardableManager takes ownership of the texture, but it otherwise
disappears from the context state. Then glLockDiscardableTextureCHROMIUM (which
would need an actual command too - the shm state still being done out-of-band)
would restore the TextureRef into the TextureManager (and it's up to the client
to rebind, etc.). We would need to make sure the client id stays "in-use" on the
client side (to avoid glGenTextures returning it again) while unlocked (which
could be a little odd, but I'm less worried about client-side).

Another option is to replace unlocked textures by a "proxy" texture in the
context state - i.e. have glUnlockDiscardableTextureCHROMIUM be somewhat
semantically equivalent to an atomic glDeleteTextures(1, &texture_id);
glBindTexture(target, texture_id); - that is, the texture is recreated without
any image attached to it. Then glLockDiscardableTextureCHROMIUM would (if it
succeeds) semantically restore the images. This could probably be achieved by
swapping the TextureRef in the TextureManager.


Either way, what I'm asking is that we define the context state wrt the
texture_id after glUnlockDiscardableTextureCHROMIUM (so that we can reason about
the interactions with other commands), and that it be not modified by the
(asynchronous) eviction.

[ericrk, 2017/05/10 21:36:02]

Makes perfect sense. I *think* I have achieved this by unbinding the texture and
transferring the TextureRef from the TextureManager > DiscardableManager when we
unlock a texture.

Note that this requires a separate service-side refcount (different form the one
on the discardable handle), as the discardable handle one may be out-of-sync
with the command buffer (we may have re-locked on the client before the re-lock
hits the GPU proc).

+    it = entries_.Erase(it);
+  }
+}
+
+bool ServiceDiscardableManager::IsEntryLockedForTesting(
+    uint32_t texture_id,
+    const gles2::ContextGroup* context_group) const {
+  auto found = entries_.Peek({texture_id, context_group});
+  DCHECK(found != entries_.end());
+
+  return found->second.handle.IsLockedForTesting();
+}
+
+}  // namespace gpu