Global GPU memory manager for android webview

android_webview/browser/browser_view_renderer.cc

Index: android_webview/browser/browser_view_renderer.cc
diff --git a/android_webview/browser/browser_view_renderer.cc b/android_webview/browser/browser_view_renderer.cc
index 937ed13259c1de75bde6d7c190b38566373c5351..48c2a3e45b96387663584e2ab0733f9dca879bfb 100644
--- a/android_webview/browser/browser_view_renderer.cc
+++ b/android_webview/browser/browser_view_renderer.cc
@@ -13,6 +13,7 @@
 #include "base/logging.h"
 #include "base/strings/stringprintf.h"
 #include "content/public/browser/android/synchronous_compositor.h"
+#include "content/public/browser/browser_thread.h"
 #include "content/public/browser/web_contents.h"
 #include "third_party/skia/include/core/SkBitmap.h"
 #include "third_party/skia/include/core/SkCanvas.h"
@@ -22,13 +23,26 @@
 using base::android::AttachCurrentThread;
 using base::android::JavaRef;
 using base::android::ScopedJavaLocalRef;
-
+using content::BrowserThread;
 namespace android_webview {
 
 namespace {
 
 const int64 kFallbackTickTimeoutInMilliseconds = 20;
 
+// Used to calculate memory and resource allocation. Determined experimentally.
+const size_t kMemoryMultiplier = 10;
+const size_t kNumGrallocLimit = 150;
+const size_t kBytesPerPixel = 4;
+const size_t kMemoryAllocationStep = 5 * 1024 * 1024;
+// Needs |kNumGrallocLimit| tiles to draw quickly, but it will be resource
+// hungry if it only gets allocated less than |kGrallocHungryLimit|.
+const size_t kGrallocHungryLimit = 100;
+// Becomes starved if the resource request cannot be granted kStarvedTimes in a

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

heh, hungry -> starved, actually makes sense

+// row.
+const size_t kStarvedTimes = 5;
+
+typedef content::SynchronousCompositorMemoryPolicy Policy;

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

Never do this. This is just confusing. If you want to save typing "content::",
you can use "using".

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

Done.

 class AutoResetWithLock {
  public:
   AutoResetWithLock(gfx::Vector2dF* scoped_variable,
@@ -79,16 +93,88 @@ BrowserViewRenderer::BrowserViewRenderer(
       compositor_needs_continuous_invalidate_(false),
       block_invalidates_(false),
       width_(0),
-      height_(0) {
+      height_(0),
+      num_hungry_(0) {
   CHECK(web_contents_);
   content::SynchronousCompositor::SetClientForWebContents(web_contents_, this);
-
   // Currently the logic in this class relies on |has_compositor_| remaining
   // false until the DidInitializeCompositor() call, hence it is not set here.
 }
 
 BrowserViewRenderer::~BrowserViewRenderer() {
   content::SynchronousCompositor::SetClientForWebContents(web_contents_, NULL);
+  Policy zero_policy;
+  SetMemoryPolicy(zero_policy);

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

This should already happen in OnDetached, not needed here

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

I'm just being safe here.
Why is OnDetached guaranteed to be called before destruction? Is it a contract
enforced by Android Framework?

Anyways, putting a DCHECK in DidDestroyCompositor should do it.
On 2014/04/28 22:22:15, boliu wrote:

[boliu, 2014/05/01 00:37:40]

First, webview is never gc-ed before it is detached. So far looks like that
assumption holds. We destroy native when we are gc-ed, so detach before
destruction is guaranteed.

The snag is that we also destroy native side when app calls Webview.destroy(),
and that was a problem for us before. But if you look at the implementation of
destroy, if destroy is called while the view is attached, then we don't actually
destroy native, but wait until we are detached to do it.

So yes, the detach before destroy must hold, otherwise we never get a chance to
clean up gl resources

+}
+
+// This function is supposed to be called after HardwareRenderer trims the
+// memory of the compositor.
+// It will update the cached memory policy in shared renderer state, as well as
+// the tile resource allocation in GlobalTileManager.
+void BrowserViewRenderer::TrimMemory() {

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

Is TileManager aware that we are dropping tiles on our own?

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

You mean global tile manager?
Yes it is aware of it, because "setMemoryPolicy" will notify the global tile
manager of this.
On 2014/04/28 22:22:15, boliu wrote:

+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  Policy zero_policy;
+  SetMemoryPolicy(zero_policy);
+  ForceFakeCompositeSW();
+  ForceDropTiles();

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

Isn't ForceDropTiles enough? Why both?

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

Yes it is enough. Removed.
On 2014/04/28 22:22:15, boliu wrote:

+}
+
+Policy BrowserViewRenderer::CalculateMemoryPolicy() {

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

IdealPolicy? Or DesiredPolicy?

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

I changed the function name to CalculateDesiredMemoryPolicy
On 2014/04/28 22:22:15, boliu wrote:

+  Policy policy;
+  policy.bytes_limit = kMemoryMultiplier * kBytesPerPixel *
+                       draw_gl_input_.global_visible_rect.width() *
+                       draw_gl_input_.global_visible_rect.height();
+  // Round up to a multiple of kMemoryAllocationStep.
+  policy.bytes_limit =
+      (policy.bytes_limit / kMemoryAllocationStep + 1) * kMemoryAllocationStep;
+  policy.num_resources_limit = CalculateTileRequest();
+  return policy;
+}
+
+// This function updates the cached memory policy in shared renderer state, as
+// well as the tile resource allocation in GlobalTileManager.
+void BrowserViewRenderer::SetMemoryPolicy(Policy& new_policy) {

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

SetMemoryPolicy sounds like a simple setter, this is doing way more.

How about RequestMemoryPolicy?

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

Good point (although I put some comments there to explain what this function
does)
On 2014/04/28 22:22:15, boliu wrote:

+  Policy current_policy = shared_renderer_state_->GetMemoryPolicy();
+  GlobalTileManager* manager = GlobalTileManager::GetInstance();
+
+  size_t num_tiles = manager->RequestTiles(current_policy.num_resources_limit,
+      new_policy.num_resources_limit, IsStarved(), tile_manager_key_);
+  if (num_tiles <
+      std::min(kGrallocHungryLimit, new_policy.num_resources_limit)) {
+    num_hungry_++;

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

this should only be modified in OnDraw. Too easy to add a call to this, and then
suddenly your policy changed.

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

SetMemoryPolicy is called in 3 places: OnDraw, TrimMemory, and
DetachedFromWindow
The latter 2 cases will just set memory policy to 0 and reset num_hungry_ to 0.

Only OnDraw will potentially request some more memory and make the BVR hungry.
On 2014/04/28 22:22:15, boliu wrote:

+  } else {
+    num_hungry_ = 0;
+  }
+
+  new_policy.num_resources_limit = num_tiles;
+  shared_renderer_state_->SetMemoryPolicy(new_policy);
+}
+
+size_t BrowserViewRenderer::CalculateTileRequest() {
+  return kNumGrallocLimit;
+}
+
+bool BrowserViewRenderer::IsStarved() {
+  // If the hardware renderer is hungry for kStarvedTimes in a row, this
+  // hardware renderer is considered starved.
+  return num_hungry_ >= kStarvedTimes;
+}
+
+size_t BrowserViewRenderer::ForceDropTiles() {
+  size_t dropped_tiles =
+      shared_renderer_state_->GetMemoryPolicy().num_resources_limit;
+  Policy zero_policy;
+
+  shared_renderer_state_->SetMemoryPolicy(zero_policy);
+  shared_renderer_state_->GetCompositor()->
+      SetMemoryPolicy(zero_policy);
+
+  // Now force a fake SW draw to make sure the tiles are dropped because the
+  // tile manager only manages tiles after a Draw.
+  // Making a fake SW draw is a hacky way to forcibly drop tiles. Maybe we can
+  // find better implementations.
+  ForceFakeCompositeSW();
+  return dropped_tiles;
 }
 
 bool BrowserViewRenderer::OnDraw(jobject java_canvas,
@@ -105,10 +191,20 @@ bool BrowserViewRenderer::OnDraw(jobject java_canvas,
     return false;
   if (is_hardware_canvas && attached_to_window_) {
     shared_renderer_state_->SetDrawGLInput(draw_gl_input_);
+
+    Policy old_policy = shared_renderer_state_->GetMemoryPolicy();
+    Policy new_policy = CalculateMemoryPolicy();
+    SetMemoryPolicy(new_policy);
     // We should be performing a hardware draw here. If we don't have the
     // compositor yet or if RequestDrawGL fails, it means we failed this draw
     // and thus return false here to clear to background color for this draw.
-    return has_compositor_ && client_->RequestDrawGL(java_canvas);
+    bool did_draw_gl = has_compositor_ && client_->RequestDrawGL(java_canvas);
+    if (did_draw_gl)
+      GlobalTileManager::GetInstance()->DidDrawGL(tile_manager_key_);

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

Hmm, why do You you need to condition this on did_draw_gl?

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

I want to be accurate about the MRU inside GlobalTileManager. The most recently
drawn view should be the head of MRU. DidDrawGL will put "this" BVR to the head
of the MRU. And that should only happen if DrawGL actually succeeded.
On 2014/04/28 22:22:15, boliu wrote:

+    else
+      SetMemoryPolicy(old_policy);

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

Why set back to old policy?

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

The memory policy is supposedly read by HardwareRenderer, if the
HardwareRenderer indeed make a DrawGL. I don't want to change the memory policy
if the HardwareRenderer did not actually DrawGL.
On 2014/04/28 22:22:15, boliu wrote:

[boliu, 2014/05/01 00:37:40]

Let's assume that never happens, that if HardwareRenderer is up, then
did_draw_gl_ must be true.

What we are really missing is a commit, ie a function copies everything HR *if
and only if* HR is about to draw the new frame. I think we'll get that
eventually

+
+    return did_draw_gl;
   }
   // Perform a software draw
   return DrawSWInternal(java_canvas, clip);
@@ -230,11 +326,20 @@ void BrowserViewRenderer::OnAttachedToWindow(int width, int height) {
   attached_to_window_ = true;
   width_ = width;
   height_ = height;
+  tile_manager_key_ = GlobalTileManager::GetInstance()->PushBack(this);
+  num_hungry_ = 0;
 }
 
 void BrowserViewRenderer::OnDetachedFromWindow() {
   TRACE_EVENT0("android_webview", "BrowserViewRenderer::OnDetachedFromWindow");
   attached_to_window_ = false;
+  Policy zero_policy;
+  SetMemoryPolicy(zero_policy);
+  GlobalTileManager::GetInstance()->Remove(tile_manager_key_);
+  num_hungry_ = 0;
+  // The hardware resources are released in the destructor of hardware renderer,
+  // so we don't need to do it here.
+  // See AwContents::ReleaseHardwareDrawOnRenderThread(JNIEnv*, jobject).
 }
 
 bool BrowserViewRenderer::IsAttachedToWindow() const {