Global GPU memory manager for android webview

android_webview/browser/global_tile_manager.cc

+// Copyright 2014 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 "android_webview/browser/global_tile_manager.h"
+#include "android_webview/browser/tile_resource_consumer.h"
+#include "base/lazy_instance.h"
+
+namespace android_webview {
+
+namespace {
+  base::LazyInstance<GlobalTileManager>::Leaky g_tile_manager =

[boliu, 2014/04/28 22:22:15]

no indent for namespace

[hush (inactive), 2014/04/30 20:50:17]

Done.

+      LAZY_INSTANCE_INITIALIZER;

[boliu, 2014/04/28 22:22:15]

blank line below

[hush (inactive), 2014/04/30 20:50:17]

Done.

+  // The soft limit of the number of file descriptors per process is 1024 on
+  // Android and gralloc buffers may not be the only thing that uses file
+  // descriptors.  For each tile, there is a gralloc buffer backing it, which
+  // uses 2 FDs.
+  const size_t kNumTilesLimit = 450;
+} // namespace
+
+// static
+GlobalTileManager* GlobalTileManager::GetInstance() {
+  return g_tile_manager.Pointer();
+}
+
+void GlobalTileManager::DidDrawGL(Key key) {

[boliu, 2014/04/28 22:22:15]

Call this DidDraw, or DidUse. And group the code together with PushBack.

[hush (inactive), 2014/04/30 20:50:17]

Done.

+  DCHECK(mru_list_.end() != key);
+
+  inactive_views_ = key;
+  inactive_views_++;

[boliu, 2014/04/28 22:22:15]

This is not what I described, you are only moving inactive_views_ to the back of
the list, treating everything that was ever done a hardware OnDraw as active. We
want views that have stopped OnDraw to eventually become inactive.

What you want is *before* moving the current view to the front, check how many
views are in front of it. Add 1 to include the view itself, that's the number of
active views (yes, it will be different for each view)

[hush (inactive), 2014/04/30 20:50:17]

This is essentially the same. Talked in person
On 2014/04/28 22:22:15, boliu wrote:

+  mru_list_.splice(mru_list_.begin(), mru_list_, key);
+}
+
+void GlobalTileManager::Remove(Key key) {
+  DCHECK(mru_list_.end() != key);
+  if (key == inactive_views_)
+    inactive_views_++;
+  mru_list_.erase(key);
+}
+
+size_t GlobalTileManager::EvictUntilSatisfied(
+    size_t desired_num_tiles, Key key) {
+  size_t total_evicted_tiles = 0;
+  ListType::reverse_iterator it;
+
+  // Evicts from the least recent drawn view, until the disired number of tiles
+  // can be reclaimed.
+  for (it = mru_list_.rbegin(); it != mru_list_.rend(); it++) {

[boliu, 2014/04/28 22:22:15]

Shouldn't you begin at something like inactive_views_?

[hush (inactive), 2014/04/30 20:50:17]

Yes.
On 2014/04/28 22:22:15, boliu wrote:

+    // key represents the view that requested the eviction, so we don't need to
+    // evict the requester itself.
+    if (*it == *key)
+      continue;
+
+    size_t evicted_tiles = (*it)->ForceDropTiles();
+    total_evicted_tiles += evicted_tiles;
+    if (total_evicted_tiles >= desired_num_tiles) {
+      break;
+    }
+  }
+
+  return total_evicted_tiles;
+}
+
+size_t GlobalTileManager::RequestTiles(size_t old_num_of_tiles,
+    size_t new_num_of_tiles, bool is_starved, Key key) {

[boliu, 2014/04/28 22:22:15]

ditto on multi-line argument style

[hush (inactive), 2014/04/30 20:50:17]

Done.

+  size_t num_of_active_views =
+      std::distance(mru_list_.begin(), inactive_views_);
+  size_t tiles_per_view_limit;
+  if (num_of_active_views == 0)
+    tiles_per_view_limit = kNumTilesLimit;
+  else
+    tiles_per_view_limit = kNumTilesLimit / num_of_active_views;
+  new_num_of_tiles = std::min(new_num_of_tiles, tiles_per_view_limit);
+  size_t tiles = allocated_tiles_ - old_num_of_tiles + new_num_of_tiles;

[boliu, 2014/04/28 22:22:15]

s/tiles/new_total_allocated_tiles/

[hush (inactive), 2014/04/30 20:50:17]

Done.

+  // Has enough tiles to satisfy the request.
+  if (tiles <= kNumTilesLimit) {
+    allocated_tiles_ = tiles;

[boliu, 2014/04/28 22:22:15]

Because individual view policy is inside each view, this class should have
DCHECKs that allocated_tiles_ remains the sum of everything. I guess add up
everything in a for loop, and only do it if DCHECK_IS_ON is set.

[hush (inactive), 2014/04/30 20:50:17]

Done.

+    return new_num_of_tiles;
+  }
+
+  // Does not have enough tiles, but the request is not starved. Gives what's
+  // left to the requester.
+  if (!is_starved) {

[boliu, 2014/04/28 22:22:15]

The staved concept does not help with stability at all. If there are 10 views
drawing, then each view can get at most 450 / 10 = 45 tiles, which is below the
100. After 5 cycles, the views are going to start killing each other.

I think you should only drop inactive views.

[hush (inactive), 2014/04/30 20:50:17]

We talked about this in person...
On 2014/04/28 22:22:15, boliu wrote:

+    size_t tiles_left = kNumTilesLimit - allocated_tiles_;
+    allocated_tiles_ = kNumTilesLimit;
+    return tiles_left + old_num_of_tiles;
+  }
+
+  // Does not have enough tiles and the requester is starved.
+  size_t evicted_tiles = EvictUntilSatisfied(tiles - kNumTilesLimit, key);
+  tiles -= evicted_tiles;
+  allocated_tiles_ = tiles;
+  return new_num_of_tiles;
+}
+
+GlobalTileManager::Key GlobalTileManager::PushBack(
+    TileResourceConsumer* tile_resource_consumer) {
+  DCHECK(mru_list_.end() ==
+         std::find(mru_list_.begin(), mru_list_.end(), tile_resource_consumer));
+  mru_list_.push_back(tile_resource_consumer);
+  Key back = mru_list_.end();
+  back--;
+  return back;
+}
+
+GlobalTileManager::GlobalTileManager() {
+  allocated_tiles_ = 0;
+  inactive_views_ = mru_list_.end();
+}
+
+GlobalTileManager::~GlobalTileManager() {}
+
+} // namespace webview