cc: Remove resource updates from scheduler.

cc/CCTextureUpdateController.cpp

Index: cc/CCTextureUpdateController.cpp
diff --git a/cc/CCTextureUpdateController.cpp b/cc/CCTextureUpdateController.cpp
index ca49d77ff3c554b013865350d5808228724468a1..bbf2b07f12fd32979996a6fb65ebfb5eced0760d 100644
--- a/cc/CCTextureUpdateController.cpp
+++ b/cc/CCTextureUpdateController.cpp
@@ -9,16 +9,20 @@
 #include "GraphicsContext3D.h"
 #include "TextureCopier.h"
 #include "TextureUploader.h"
-#include <wtf/CurrentTime.h>
+
+using namespace std;

[jamesr, 2012/09/14 23:03:06]

chromium and WebKit style is to prefer std::foo instead of this using directive

[reveman, 2012/09/14 23:38:00]

Hm, I see a lot of the using directive in cc. I thought we preferred this at
least in WebKit. Anyhow, I'll fix this.

[jamesr, 2012/09/17 06:57:38]

This changed in WebKit several months ago:
http://www.webkit.org/coding/coding-style.html#using-in-cpp . A fair amount of
code is still using the old WebKit style, but we shouldn't propagate it further.

[reveman, 2012/09/17 15:32:52]

ok, clear.

 
 namespace {
 
-// Number of textures to update with each call to updateMoreTexturesIfEnoughTimeRemaining().
+// Number of textures to update with each call to updateMoreTextures().
 static const size_t textureUpdatesPerTick = 12;
 
 // Measured in seconds.
 static const double textureUpdateTickRate = 0.004;
 
+// Measured in seconds.
+static const double uploaderBusyTickRate = 0.001;
+
 // Flush interval when performing texture uploads.
 static const int textureUploadFlushPeriod = 4;
 
@@ -99,8 +103,6 @@ CCTextureUpdateController::CCTextureUpdateController(CCTextureUpdateControllerCl
     , m_resourceProvider(resourceProvider)
     , m_copier(copier)
     , m_uploader(uploader)
-    , m_monotonicTimeLimit(0)
-    , m_firstUpdateAttempt(true)
 {
 }
 
@@ -108,40 +110,30 @@ CCTextureUpdateController::~CCTextureUpdateController()
 {
 }
 
-void CCTextureUpdateController::updateMoreTextures(double monotonicTimeLimit)
+void CCTextureUpdateController::start()
 {
-    ASSERT(monotonicTimeLimit >= m_monotonicTimeLimit);
-    m_monotonicTimeLimit = monotonicTimeLimit;
+    // Post a 0-delay task when no updates were left. When it runs,
+    // updateTexturesCompleted() will be called.
+    if (!updateMoreTextures())
+        m_timer->startOneShot(0);
+}
 
-    // Update already in progress.
-    if (m_timer->isActive())
+void CCTextureUpdateController::updateAllTexturesNow()
+{
+    if (!m_queue->hasMoreUpdates())
         return;
 
-    // Call updateMoreTexturesNow() directly unless it's the first update
-    // attempt. This ensures that we empty the update queue in a finite
-    // amount of time.
-    if (m_firstUpdateAttempt) {
-        // Post a 0-delay task when no updates were left. When it runs,
-        // updateTexturesCompleted() will be called.
-        if (!updateMoreTexturesIfEnoughTimeRemaining())
-            m_timer->startOneShot(0);
-
-        m_firstUpdateAttempt = false;
-    } else
-        updateMoreTexturesNow();
+    updateTextures(
+        m_resourceProvider, m_copier, m_uploader, m_queue.get(),
+        numeric_limits<size_t>::max());
 }
 
 void CCTextureUpdateController::onTimerFired()
 {
-    if (!updateMoreTexturesIfEnoughTimeRemaining())
+    if (!updateMoreTextures())
         m_client->updateTexturesCompleted();
 }
 
-double CCTextureUpdateController::monotonicTimeNow() const
-{
-    return monotonicallyIncreasingTime();
-}
-
 double CCTextureUpdateController::updateMoreTexturesTime() const
 {
     return textureUpdateTickRate;
@@ -152,22 +144,38 @@ size_t CCTextureUpdateController::updateMoreTexturesSize() const
     return textureUpdatesPerTick;
 }
 
-bool CCTextureUpdateController::updateMoreTexturesIfEnoughTimeRemaining()
+// Performs buffered texture updates. Currently only full uploads.
+bool CCTextureUpdateController::updateMoreTextures()
 {
-    if (!m_queue->hasMoreUpdates())
+    if (m_uploader->isBusy()) {
+        m_timer->startOneShot(uploaderBusyTickRate);

[jamesr, 2012/09/14 23:03:06]

why the new 1ms delay? did anything in the previous system tick this fast?

[reveman, 2012/09/14 23:38:00]

So the main reason this code is here is because we don't want to consider full
uploads done until the uploader is available. If we didn't make sure the
uploader was available, we would transition to commit and unnecessarily block
there while waiting for the uploader to become available so we can perform
partial uploads.

we could use the tick rate as delay here but using a shorter delay significantly
reduces the amount of unnecessary idle time on the impl during the
beginFrameComplete to commit cycle.

I'll add a comment to the code here if nothing else.

[jamesr, 2012/09/17 06:57:38]

This is all because we poll (via the query object) to see if the uploader's
ready instead of getting a callback, right?

I'm a bit worried about hammering the system with really fast timeouts - it adds
a lot of variability and stress to the scheduler and the rest of the system. 
What's the common case here?  I would guess a small number of full uploads
(sometimes zero) and a small number of partial uploads (sometimes zero).  Are we
going to end up spinning on this every time we do any buffered full uploads? 
How often do you expect this to spin?

For the case you mention of partial uploads I think as soon as we decide to do
partial uploads there's not much point blocking on the client side - if we're at
the point where we're ready to upload partials we should just go ahead and
buffer the frame up.  We can't really usefully put any frames up with the old
textures once we've begun the partial uploads.

[reveman, 2012/09/17 15:32:52]

Yes, it's because we poll.
On a system where the upload estimate is accurate we rarely end up spinning
here. We do one 1ms timeout a little less than 50% of the time when performing
12+ uploads (textureUploadsPerTick=12).

In general we spin more often when upload time estimate is too aggressive and
less when it's set too conservatively.
That's what this patch does. Partial uploads are done as part of the commit step
and we never wait for them to complete before we queue a draw.

+        return true;
+    }
+
+    if (!m_queue->fullUploadSize())
         return false;
 
-    bool hasTimeRemaining = monotonicTimeNow() < m_monotonicTimeLimit - updateMoreTexturesTime();
-    if (hasTimeRemaining)
-        updateMoreTexturesNow();
+    size_t uploads = min(m_queue->fullUploadSize(), updateMoreTexturesSize());
+    m_timer->startOneShot(
+        updateMoreTexturesTime() / updateMoreTexturesSize() * uploads);
 
-    return true;
-}
+    m_uploader->beginUploads();
 
-void CCTextureUpdateController::updateMoreTexturesNow()
-{
-    m_timer->startOneShot(updateMoreTexturesTime());
-    updateTextures(m_resourceProvider, m_copier, m_uploader, m_queue.get(), updateMoreTexturesSize());
+    size_t uploadCount = 0;
+    while (uploads--) {
+      m_uploader->uploadTexture(
+          m_resourceProvider, m_queue->takeFirstFullUpload());
+      uploadCount++;
+      if (!(uploadCount % textureUploadFlushPeriod))
+          m_resourceProvider->shallowFlushIfSupported();
+    }
+
+    // Make sure there are no dangling partial uploads without a flush.
+    if (uploadCount % textureUploadFlushPeriod)
+        m_resourceProvider->shallowFlushIfSupported();
+
+    m_uploader->endUploads();
+    return true;
 }
 
 }