[BlackBerry] Replace the three different rendering mechanisms for clearing the render...
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Aug 2012 21:53:37 +0000 (21:53 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Aug 2012 21:53:37 +0000 (21:53 +0000)
https://bugs.webkit.org/show_bug.cgi?id=94837

Patch by Adam Treat <atreat@rim.com> on 2012-08-23
Reviewed by George Staikos.

PR 197738

Currently, we have three different mechanisms for clearing the render queue.
The first mechanism is render on idle.  Whenever the webkit thread becomes idle
(read: no more events in its queue) we render the next job in the render queue.
 This is the primary means we use for clearing the render queue.  However, this
mechanism has a flaw, it is such a low priority mechanism that sometimes the
queue grows so fast due to higher priority events adding rects to the queue
that this mechanism can't possibly keep up.  That is what leads to the second
mechanism: rendering right before a timer is fired when we discover that the
render queue is under pressure and rendering on idle can't keep up.  However,
there are still degenerate cases where even this mechanism does not allow us to
keep up.  That brings us to the third mechanism: rendering based on a timer
that is a catch-all.

The second and third mechanisms lead to very large render jobs as they try and
clear the queue faster when it comes under pressure.  These very large render
jobs end up keeping the webkit thread busy with a message that can take large
fractions of a second to resolve.

These three mechanisms were put in place when the backingstore had a different
overall design that was not truly asynchronous.  This patch replaces these
three mechanisms with a single one that uses the platform messaging classes to
full purpose - a uniquely coalescing message that has a higher priority level
than timers making sure the render queue can never come under pressure.

* Api/BackingStore.cpp:
(BlackBerry::WebKit::BackingStorePrivate::BackingStorePrivate):
(WebKit):
(RenderJobMessage):
(BlackBerry::WebKit::RenderJobMessage::RenderJobMessage):
(BlackBerry::WebKit::BackingStorePrivate::dispatchRenderJob):
(BlackBerry::WebKit::BackingStorePrivate::renderJob):
(BlackBerry::WebKit::BackingStore::blitContents):
* Api/BackingStore.h:
* Api/BackingStore_p.h:
(BackingStorePrivate):
* Api/WebPage.cpp:
* Api/WebPage.h:
* WebKitSupport/RenderQueue.cpp:
(BlackBerry::WebKit::RenderQueue::reset):
(BlackBerry::WebKit::RenderQueue::addToRegularQueue):
(BlackBerry::WebKit::RenderQueue::addToScrollZoomQueue):
(BlackBerry::WebKit::RenderQueue::clear):
(BlackBerry::WebKit::RenderQueue::clearVisibleZoom):
(BlackBerry::WebKit::RenderQueue::render):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@126485 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebKit/blackberry/Api/BackingStore.cpp
Source/WebKit/blackberry/Api/BackingStore.h
Source/WebKit/blackberry/Api/BackingStore_p.h
Source/WebKit/blackberry/Api/WebPage.cpp
Source/WebKit/blackberry/Api/WebPage.h
Source/WebKit/blackberry/ChangeLog
Source/WebKit/blackberry/WebKitSupport/RenderQueue.cpp

index 8786650..b86cc5d 100644 (file)
@@ -75,7 +75,6 @@ using BlackBerry::Platform::IntSize;
 namespace BlackBerry {
 namespace WebKit {
 
-const int s_renderTimerTimeout = 1.0;
 WebPage* BackingStorePrivate::s_currentBackingStoreOwner = 0;
 
 typedef std::pair<int, int> Divisor;
@@ -217,8 +216,6 @@ BackingStorePrivate::BackingStorePrivate()
     m_frontState = reinterpret_cast<unsigned>(new BackingStoreGeometry);
     m_backState = reinterpret_cast<unsigned>(new BackingStoreGeometry);
 
-    m_renderTimer = adoptPtr(new Timer<BackingStorePrivate>(this, &BackingStorePrivate::renderOnTimer));
-
     // Need a recursive mutex to achieve a global lock.
     pthread_mutexattr_t attr;
     pthread_mutexattr_init(&attr);
@@ -500,62 +497,27 @@ bool BackingStorePrivate::shouldPerformRegularRenderJobs() const
     return shouldPerformRenderJobs() && !m_suspendRegularRenderJobs;
 }
 
-void BackingStorePrivate::startRenderTimer()
-{
-    // Called when render queue has a new job added.
-    if (m_renderTimer->isActive() || m_renderQueue->isEmpty(!m_suspendRegularRenderJobs))
-        return;
-
-#if DEBUG_BACKINGSTORE
-    BlackBerry::Platform::log(BlackBerry::Platform::LogLevelCritical, "BackingStorePrivate::startRenderTimer time=%f", WTF::currentTime());
-#endif
-    m_renderTimer->startOneShot(s_renderTimerTimeout);
-}
-
-void BackingStorePrivate::stopRenderTimer()
+static const BlackBerry::Platform::Message::Type RenderJobMessageType = BlackBerry::Platform::Message::generateUniqueMessageType();
+class RenderJobMessage : public BlackBerry::Platform::ExecutableMessage
 {
-    if (!m_renderTimer->isActive())
-        return;
-
-    // Called when we render something to restart.
-#if DEBUG_BACKINGSTORE
-    BlackBerry::Platform::log(BlackBerry::Platform::LogLevelCritical, "BackingStorePrivate::stopRenderTimer time=%f", WTF::currentTime());
-#endif
-    m_renderTimer->stop();
-}
+public:
+    RenderJobMessage(BlackBerry::Platform::MessageDelegate* delegate)
+        : BlackBerry::Platform::ExecutableMessage(delegate, BlackBerry::Platform::ExecutableMessage::UniqueCoalescing, RenderJobMessageType)
+    { }
+};
 
-void BackingStorePrivate::renderOnTimer(WebCore::Timer<BackingStorePrivate>*)
+void BackingStorePrivate::dispatchRenderJob()
 {
-    // This timer is a third method of starting a render operation that is a catch-all. If more
-    // than s_renderTimerTimeout elapses with no rendering taking place and render jobs in the queue, then
-    // renderOnTimer will be called which will actually render.
-    if (!shouldPerformRenderJobs())
-        return;
-
-#if DEBUG_BACKINGSTORE
-    BlackBerry::Platform::log(BlackBerry::Platform::LogLevelCritical, "BackingStorePrivate::renderOnTimer time=%f", WTF::currentTime());
-#endif
-    while (m_renderQueue->hasCurrentVisibleZoomJob() || m_renderQueue->hasCurrentVisibleScrollJob())
-        m_renderQueue->render(!m_suspendRegularRenderJobs);
-
-    if (shouldPerformRegularRenderJobs() && m_renderQueue->hasCurrentRegularRenderJob())
-        m_renderQueue->renderAllCurrentRegularRenderJobs();
-
-#if USE(ACCELERATED_COMPOSITING)
-    drawLayersOnCommitIfNeeded();
-#endif
+    BlackBerry::Platform::MessageDelegate* messageDelegate = BlackBerry::Platform::createMethodDelegate(&BackingStorePrivate::renderJob, this);
+    BlackBerry::Platform::webKitThreadMessageClient()->dispatchMessage(new RenderJobMessage(messageDelegate));
 }
 
-void BackingStorePrivate::renderOnIdle()
+void BackingStorePrivate::renderJob()
 {
     ASSERT(shouldPerformRenderJobs());
 
-    // Let the render queue know that we entered a new event queue cycle
-    // so it can determine if it is under pressure.
-    m_renderQueue->eventQueueCycled();
-
 #if DEBUG_BACKINGSTORE
-    BlackBerry::Platform::log(BlackBerry::Platform::LogLevelCritical, "BackingStorePrivate::renderOnIdle");
+    BlackBerry::Platform::logAlways(BlackBerry::Platform::LogLevelCritical, "BackingStorePrivate::renderJob");
 #endif
 
     m_renderQueue->render(!m_suspendRegularRenderJobs);
@@ -563,41 +525,9 @@ void BackingStorePrivate::renderOnIdle()
 #if USE(ACCELERATED_COMPOSITING)
     drawLayersOnCommitIfNeeded();
 #endif
-}
-
-bool BackingStorePrivate::willFireTimer()
-{
-    // Let the render queue know that we entered a new event queue cycle
-    // so it can determine if it is under pressure.
-    m_renderQueue->eventQueueCycled();
-
-    if (!shouldPerformRegularRenderJobs() || !m_renderQueue->hasCurrentRegularRenderJob() || !m_renderQueue->currentRegularRenderJobBatchUnderPressure())
-        return true;
-
-#if DEBUG_BACKINGSTORE
-    BlackBerry::Platform::log(BlackBerry::Platform::LogLevelCritical, "BackingStorePrivate::willFireTimer");
-#endif
-
-    // We've detected that the regular render jobs are coming under pressure likely
-    // due to timers firing producing invalidation jobs and our efforts to break them
-    // up into bite size pieces has produced a situation where we can not complete
-    // a batch of them before receiving more that intersect them which causes us
-    // to start the batch over. To mitigate this we have to empty the current batch
-    // when this is detected.
-
-    // We still want to perform priority jobs first to avoid redundant paints.
-    while (m_renderQueue->hasCurrentVisibleZoomJob() || m_renderQueue->hasCurrentVisibleScrollJob())
-        m_renderQueue->render(!m_suspendRegularRenderJobs);
-
-    if (m_renderQueue->hasCurrentRegularRenderJob())
-        m_renderQueue->renderAllCurrentRegularRenderJobs();
 
-#if USE(ACCELERATED_COMPOSITING)
-    drawLayersOnCommitIfNeeded();
-#endif
-
-    // Let the caller yield and reschedule the timer.
-    return false;
+    if (shouldPerformRenderJobs())
+        dispatchRenderJob();
 }
 
 Platform::IntRect BackingStorePrivate::expandedContentsRect() const
@@ -2790,16 +2720,6 @@ void BackingStore::repaint(int x, int y, int width, int height,
     d->repaint(Platform::IntRect(x, y, width, height), contentChanged, immediate);
 }
 
-bool BackingStore::hasRenderJobs() const
-{
-    return d->shouldPerformRenderJobs();
-}
-
-void BackingStore::renderOnIdle()
-{
-    d->renderOnIdle();
-}
-
 bool BackingStore::isDirectRenderingToWindow() const
 {
     BackingStoreMutexLocker locker(d);
index 6ac9c38..c3185dc 100644 (file)
@@ -63,9 +63,6 @@ public:
     void blitContents(const BlackBerry::Platform::IntRect& dstRect, const BlackBerry::Platform::IntRect& contents);
     void repaint(int x, int y, int width, int height, bool contentChanged, bool immediate);
 
-    bool hasRenderJobs() const;
-    void renderOnIdle();
-
     // In the defers blit mode, any blit requests will just return early, and
     // a blit job will be queued that is executed by calling blitOnIdle().
     bool defersBlit() const;
index c296847..77eeab9 100644 (file)
@@ -131,11 +131,8 @@ public:
     bool shouldSuppressNonVisibleRegularRenderJobs() const;
     bool shouldPerformRenderJobs() const;
     bool shouldPerformRegularRenderJobs() const;
-    void startRenderTimer();
-    void stopRenderTimer();
-    void renderOnTimer(WebCore::Timer<BackingStorePrivate>*);
-    void renderOnIdle();
-    bool willFireTimer();
+    void dispatchRenderJob();
+    void renderJob();
 
     // Set of helper methods for the scrollBackingStore() method.
     Platform::IntRect contentsRect() const;
@@ -364,9 +361,6 @@ public:
 
     Platform::IntRect m_visibleTileBufferRect;
 
-    // Last resort timer for rendering.
-    OwnPtr<WebCore::Timer<BackingStorePrivate> > m_renderTimer;
-
     pthread_mutex_t m_mutex;
 
 #if USE(ACCELERATED_COMPOSITING)
index 3a0cf62..f73a412 100644 (file)
@@ -5292,14 +5292,6 @@ bool WebPage::defersLoading() const
     return d->m_page->defersLoading();
 }
 
-bool WebPage::willFireTimer()
-{
-    if (d->isLoading())
-        return true;
-
-    return d->m_backingStore->d->willFireTimer();
-}
-
 void WebPage::notifyPagePause()
 {
     FOR_EACH_PLUGINVIEW(d->m_pluginViews)
index dc4cf16..9294b86 100644 (file)
@@ -322,8 +322,6 @@ public:
 
     bool defersLoading() const;
 
-    bool willFireTimer();
-
     bool isEnableLocalAccessToAllCookies() const;
     void setEnableLocalAccessToAllCookies(bool);
 
index 398e96e..1cde6c7 100644 (file)
@@ -1,3 +1,57 @@
+2012-08-23  Adam Treat  <atreat@rim.com>
+
+        [BlackBerry] Replace the three different rendering mechanisms for clearing the render queue
+        https://bugs.webkit.org/show_bug.cgi?id=94837
+
+        Reviewed by George Staikos.
+
+        PR 197738
+
+        Currently, we have three different mechanisms for clearing the render queue.
+        The first mechanism is render on idle.  Whenever the webkit thread becomes idle
+        (read: no more events in its queue) we render the next job in the render queue.
+         This is the primary means we use for clearing the render queue.  However, this
+        mechanism has a flaw, it is such a low priority mechanism that sometimes the
+        queue grows so fast due to higher priority events adding rects to the queue
+        that this mechanism can't possibly keep up.  That is what leads to the second
+        mechanism: rendering right before a timer is fired when we discover that the
+        render queue is under pressure and rendering on idle can't keep up.  However,
+        there are still degenerate cases where even this mechanism does not allow us to
+        keep up.  That brings us to the third mechanism: rendering based on a timer
+        that is a catch-all.
+
+        The second and third mechanisms lead to very large render jobs as they try and
+        clear the queue faster when it comes under pressure.  These very large render
+        jobs end up keeping the webkit thread busy with a message that can take large
+        fractions of a second to resolve.
+
+        These three mechanisms were put in place when the backingstore had a different
+        overall design that was not truly asynchronous.  This patch replaces these
+        three mechanisms with a single one that uses the platform messaging classes to
+        full purpose - a uniquely coalescing message that has a higher priority level
+        than timers making sure the render queue can never come under pressure.
+
+        * Api/BackingStore.cpp:
+        (BlackBerry::WebKit::BackingStorePrivate::BackingStorePrivate):
+        (WebKit):
+        (RenderJobMessage):
+        (BlackBerry::WebKit::RenderJobMessage::RenderJobMessage):
+        (BlackBerry::WebKit::BackingStorePrivate::dispatchRenderJob):
+        (BlackBerry::WebKit::BackingStorePrivate::renderJob):
+        (BlackBerry::WebKit::BackingStore::blitContents):
+        * Api/BackingStore.h:
+        * Api/BackingStore_p.h:
+        (BackingStorePrivate):
+        * Api/WebPage.cpp:
+        * Api/WebPage.h:
+        * WebKitSupport/RenderQueue.cpp:
+        (BlackBerry::WebKit::RenderQueue::reset):
+        (BlackBerry::WebKit::RenderQueue::addToRegularQueue):
+        (BlackBerry::WebKit::RenderQueue::addToScrollZoomQueue):
+        (BlackBerry::WebKit::RenderQueue::clear):
+        (BlackBerry::WebKit::RenderQueue::clearVisibleZoom):
+        (BlackBerry::WebKit::RenderQueue::render):
+
 2012-08-23  Antonio Gomes  <agomes@rim.com>
 
         [BlackBerry] Unify slow and fast in-region scrolling code paths
index c4b02b5..3460bd8 100644 (file)
@@ -231,7 +231,6 @@ void RenderQueue::reset()
     m_currentRegularRenderJobsBatch.clear();
     m_currentRegularRenderJobsBatchRegion = Platform::IntRectRegion();
     m_regularRenderJobsNotRenderedRegion = Platform::IntRectRegion();
-    m_parent->stopRenderTimer();
     ASSERT(isEmpty());
 }
 
@@ -375,7 +374,7 @@ void RenderQueue::addToRegularQueue(const Platform::IntRect& rect)
         m_regularRenderJobsRegion = Platform::IntRectRegion::unionRegions(m_regularRenderJobsRegion, rect);
 
     if (!isEmpty())
-        m_parent->startRenderTimer(); // Start the render timer since we could have some stale content here...
+        m_parent->dispatchRenderJob();
 }
 
 void RenderQueue::addToScrollZoomQueue(const RenderRect& rect, RenderRectList* rectList)
@@ -390,7 +389,7 @@ void RenderQueue::addToScrollZoomQueue(const RenderRect& rect, RenderRectList* r
     rectList->push_back(rect);
 
     if (!isEmpty())
-        m_parent->startRenderTimer(); // Start the render timer since we know we could have some checkerboard here...
+        m_parent->dispatchRenderJob();
 }
 
 void RenderQueue::quickSort(RenderRectList* queue)
@@ -527,9 +526,6 @@ void RenderQueue::clear(const Platform::IntRect& rect, bool clearRegularRenderJo
 
     if (m_nonVisibleScrollJobs.empty() && !m_nonVisibleScrollJobsCompleted.empty())
         nonVisibleScrollJobsCompleted();
-
-    if (isEmpty())
-         m_parent->stopRenderTimer();
 }
 
 void RenderQueue::clearRegularRenderJobs(const Platform::IntRect& rect)
@@ -548,8 +544,6 @@ void RenderQueue::clearRegularRenderJobs(const Platform::IntRect& rect)
 void RenderQueue::clearVisibleZoom()
 {
     m_visibleZoomJobs.clear();
-    if (isEmpty())
-         m_parent->stopRenderTimer();
 }
 
 bool RenderQueue::regularRenderJobsPreviouslyAttemptedButNotRendered(const Platform::IntRect& rect)
@@ -590,9 +584,6 @@ void RenderQueue::render(bool shouldPerformRegularRenderJobs)
             renderRegularRenderJob();
     } else if (!m_nonVisibleScrollJobs.empty())
         renderNonVisibleScrollJob();
-
-    if (isEmpty())
-        m_parent->stopRenderTimer();
 }
 
 void RenderQueue::renderAllCurrentRegularRenderJobs()