cc: Partial tile update for one-copy raster.

cc/resources/resource_pool.cc

Index: cc/resources/resource_pool.cc
diff --git a/cc/resources/resource_pool.cc b/cc/resources/resource_pool.cc
index 22847809ba3c196f66df1112d978d56342665b3a..7e216c180f8cea5e89978b3b1a7ae2e468d39200 100644
--- a/cc/resources/resource_pool.cc
+++ b/cc/resources/resource_pool.cc
@@ -21,7 +21,8 @@ ResourcePool::ResourcePool(ResourceProvider* resource_provider, GLenum target)
 
 ResourcePool::~ResourcePool() {
   while (!busy_resources_.empty()) {
-    DidFinishUsingResource(busy_resources_.front());
+    auto const& front = busy_resources_.front();
+    DidFinishUsingResource(front.resource, front.id);
     busy_resources_.pop_front();
   }
 
@@ -37,7 +38,7 @@ scoped_ptr<ScopedResource> ResourcePool::AcquireResource(
   for (ResourceList::iterator it = unused_resources_.begin();
        it != unused_resources_.end();
        ++it) {
-    ScopedResource* resource = *it;
+    ScopedResource* resource = it->resource;
     DCHECK(resource_provider_->CanLockForWrite(resource->id()));
 
     if (resource->format() != format)
@@ -59,8 +60,32 @@ scoped_ptr<ScopedResource> ResourcePool::AcquireResource(
   return resource.Pass();
 }
 
-void ResourcePool::ReleaseResource(scoped_ptr<ScopedResource> resource) {
-  busy_resources_.push_back(resource.release());
+scoped_ptr<ScopedResource> ResourcePool::TryAcquireOldResource(
+    const gfx::Size& size,
+    ResourceFormat format,
+    uint64_t id) {
+  if (!id)
+    return nullptr;
+
+  for (auto it = unused_resources_.begin(); it != unused_resources_.end();

[piman, 2015/05/14 19:50:46]

Should we check busy_resources_ too? We only need the resource for reading, not
writing.

Or more accurately, if the resource is available for writing, then we can use it
directly, only rasterizing the damage rect, without even needing to copy.
Otherwise, if it's available, but only for reading, then we can copy from it
into a new resource.

[danakj, 2015/05/14 19:59:47]

Oh that's a good point, we don't need to copy if it's in unused at all.

[danakj, 2015/05/14 22:03:42]

This was awesome. For on-screen animations we don't need to touch
busy_resources_ so this takes the raster time from .11 to under .08ms for a
spinner now.

On chromeos if you do it just write and drag a window with a spinner to push it
off the screen, you can occasionally activate while the resource is still
uploading and then the next frame sees a busy resource. So maybe it's good to
have that too. But 99.9% (100% on desktop) of the time we hit the non-busy
resources so, win.

+       ++it) {

[piman, 2015/05/14 19:50:46]

should unused_resources_ be a hash map indexed by id?

[danakj, 2015/05/14 19:59:47]

i could also make it a base::SmallMap, i'm not actually sure how big it gets in
the ui compositor.. i'll check.

[danakj, 2015/05/14 22:03:42]

There are at most 32 things in the vector.
https://code.google.com/p/chromium/codesearch#chromium/src/cc/trees/layer_tre...

My intuition is that doesn't warrent a hash_map. WDYT?

+    const PoolResource& pr = *it;
+    if (pr.id != id || pr.resource->format() != format ||
+        pr.resource->size() != size)

[piman, 2015/05/14 19:50:46]

If id match but format/size are different, can we just early exit?
Actually should we release that old resource altogether too? Things got resized,
doesn't sound like we would typically reuse it.

And if we did left it in, then we might end up with 2 (or more) resources with
the same id, one older than the other, and the logic here could end up giving
you a resource that doesn't match the damage rect you think you need (e.g.
resize from size A to B back to A).

(I don't understand how tile ids work though, so what I'm saying possibly
doesn't make sense)

[danakj, 2015/05/14 19:59:47]

We can reuse it rather than mallocing a new one in AcquireResource() even if we
can't reuse the pixel data inside it.
The id is unique, every invalidation generates a new Tile which is a new id.

+      continue;
+
+    ScopedResource* resource = pr.resource;
+    unused_memory_usage_bytes_ -= pr.resource->bytes();
+    unused_resources_.erase(it);
+    return make_scoped_ptr(resource);
+  }
+
+  return nullptr;
+}
+
+void ResourcePool::ReleaseResource(scoped_ptr<ScopedResource> resource,
+                                   uint64_t id) {
+  busy_resources_.push_back(PoolResource(resource.release(), id));
 }
 
 void ResourcePool::SetResourceUsageLimits(size_t max_memory_usage_bytes,
@@ -85,7 +110,7 @@ void ResourcePool::ReduceResourceUsage() {
     // can't be locked for write might also not be truly free-able.
     // We can free the resource here but it doesn't mean that the
     // memory is necessarily returned to the OS.
-    ScopedResource* resource = unused_resources_.front();
+    ScopedResource* resource = unused_resources_.front().resource;
     unused_resources_.pop_front();
     memory_usage_bytes_ -= resource->bytes();
     unused_memory_usage_bytes_ -= resource->bytes();
@@ -108,13 +133,13 @@ void ResourcePool::CheckBusyResources(bool wait_if_needed) {
   ResourceList::iterator it = busy_resources_.begin();
 
   while (it != busy_resources_.end()) {
-    ScopedResource* resource = *it;
+    ScopedResource* resource = it->resource;
 
     if (wait_if_needed)
       resource_provider_->WaitReadLockIfNeeded(resource->id());
 
     if (resource_provider_->CanLockForWrite(resource->id())) {
-      DidFinishUsingResource(resource);
+      DidFinishUsingResource(resource, it->id);
       it = busy_resources_.erase(it);
     } else {
       ++it;
@@ -122,9 +147,10 @@ void ResourcePool::CheckBusyResources(bool wait_if_needed) {
   }
 }
 
-void ResourcePool::DidFinishUsingResource(ScopedResource* resource) {
+void ResourcePool::DidFinishUsingResource(ScopedResource* resource,
+                                          uint64_t id) {
   unused_memory_usage_bytes_ += resource->bytes();
-  unused_resources_.push_back(resource);
+  unused_resources_.push_back(PoolResource(resource, id));

[piman, 2015/05/14 19:50:46]

Do we need to check for duplicates here?

[danakj, 2015/05/14 19:59:47]

I don't think so, id is unique.

 }
 
 }  // namespace cc