Title: [126485] trunk/Source/WebKit/blackberry
Revision
126485
Author
[email protected]
Date
2012-08-23 14:53:37 -0700 (Thu, 23 Aug 2012)

Log Message

[BlackBerry] Replace the three different rendering mechanisms for clearing the render queue
https://bugs.webkit.org/show_bug.cgi?id=94837

Patch by Adam Treat <[email protected]> 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):

Modified Paths

Diff

Modified: trunk/Source/WebKit/blackberry/Api/BackingStore.cpp (126484 => 126485)


--- trunk/Source/WebKit/blackberry/Api/BackingStore.cpp	2012-08-23 21:45:42 UTC (rev 126484)
+++ trunk/Source/WebKit/blackberry/Api/BackingStore.cpp	2012-08-23 21:53:37 UTC (rev 126485)
@@ -75,7 +75,6 @@
 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 @@
     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 @@
     return shouldPerformRenderJobs() && !m_suspendRegularRenderJobs;
 }
 
-void BackingStorePrivate::startRenderTimer()
+static const BlackBerry::Platform::Message::Type RenderJobMessageType = BlackBerry::Platform::Message::generateUniqueMessageType();
+class RenderJobMessage : public BlackBerry::Platform::ExecutableMessage
 {
-    // Called when render queue has a new job added.
-    if (m_renderTimer->isActive() || m_renderQueue->isEmpty(!m_suspendRegularRenderJobs))
-        return;
+public:
+    RenderJobMessage(BlackBerry::Platform::MessageDelegate* delegate)
+        : BlackBerry::Platform::ExecutableMessage(delegate, BlackBerry::Platform::ExecutableMessage::UniqueCoalescing, RenderJobMessageType)
+    { }
+};
 
-#if DEBUG_BACKINGSTORE
-    BlackBerry::Platform::log(BlackBerry::Platform::LogLevelCritical, "BackingStorePrivate::startRenderTimer time=%f", WTF::currentTime());
-#endif
-    m_renderTimer->startOneShot(s_renderTimerTimeout);
-}
-
-void BackingStorePrivate::stopRenderTimer()
+void BackingStorePrivate::dispatchRenderJob()
 {
-    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();
+    BlackBerry::Platform::MessageDelegate* messageDelegate = BlackBerry::Platform::createMethodDelegate(&BackingStorePrivate::renderJob, this);
+    BlackBerry::Platform::webKitThreadMessageClient()->dispatchMessage(new RenderJobMessage(messageDelegate));
 }
 
-void BackingStorePrivate::renderOnTimer(WebCore::Timer<BackingStorePrivate>*)
+void BackingStorePrivate::renderJob()
 {
-    // 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
-}
-
-void BackingStorePrivate::renderOnIdle()
-{
     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 @@
 #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 @@
     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);

Modified: trunk/Source/WebKit/blackberry/Api/BackingStore.h (126484 => 126485)


--- trunk/Source/WebKit/blackberry/Api/BackingStore.h	2012-08-23 21:45:42 UTC (rev 126484)
+++ trunk/Source/WebKit/blackberry/Api/BackingStore.h	2012-08-23 21:53:37 UTC (rev 126485)
@@ -63,9 +63,6 @@
     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;

Modified: trunk/Source/WebKit/blackberry/Api/BackingStore_p.h (126484 => 126485)


--- trunk/Source/WebKit/blackberry/Api/BackingStore_p.h	2012-08-23 21:45:42 UTC (rev 126484)
+++ trunk/Source/WebKit/blackberry/Api/BackingStore_p.h	2012-08-23 21:53:37 UTC (rev 126485)
@@ -131,11 +131,8 @@
     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 @@
 
     Platform::IntRect m_visibleTileBufferRect;
 
-    // Last resort timer for rendering.
-    OwnPtr<WebCore::Timer<BackingStorePrivate> > m_renderTimer;
-
     pthread_mutex_t m_mutex;
 
 #if USE(ACCELERATED_COMPOSITING)

Modified: trunk/Source/WebKit/blackberry/Api/WebPage.cpp (126484 => 126485)


--- trunk/Source/WebKit/blackberry/Api/WebPage.cpp	2012-08-23 21:45:42 UTC (rev 126484)
+++ trunk/Source/WebKit/blackberry/Api/WebPage.cpp	2012-08-23 21:53:37 UTC (rev 126485)
@@ -5292,14 +5292,6 @@
     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)

Modified: trunk/Source/WebKit/blackberry/Api/WebPage.h (126484 => 126485)


--- trunk/Source/WebKit/blackberry/Api/WebPage.h	2012-08-23 21:45:42 UTC (rev 126484)
+++ trunk/Source/WebKit/blackberry/Api/WebPage.h	2012-08-23 21:53:37 UTC (rev 126485)
@@ -322,8 +322,6 @@
 
     bool defersLoading() const;
 
-    bool willFireTimer();
-
     bool isEnableLocalAccessToAllCookies() const;
     void setEnableLocalAccessToAllCookies(bool);
 

Modified: trunk/Source/WebKit/blackberry/ChangeLog (126484 => 126485)


--- trunk/Source/WebKit/blackberry/ChangeLog	2012-08-23 21:45:42 UTC (rev 126484)
+++ trunk/Source/WebKit/blackberry/ChangeLog	2012-08-23 21:53:37 UTC (rev 126485)
@@ -1,3 +1,57 @@
+2012-08-23  Adam Treat  <[email protected]>
+
+        [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  <[email protected]>
 
         [BlackBerry] Unify slow and fast in-region scrolling code paths

Modified: trunk/Source/WebKit/blackberry/WebKitSupport/RenderQueue.cpp (126484 => 126485)


--- trunk/Source/WebKit/blackberry/WebKitSupport/RenderQueue.cpp	2012-08-23 21:45:42 UTC (rev 126484)
+++ trunk/Source/WebKit/blackberry/WebKitSupport/RenderQueue.cpp	2012-08-23 21:53:37 UTC (rev 126485)
@@ -231,7 +231,6 @@
     m_currentRegularRenderJobsBatch.clear();
     m_currentRegularRenderJobsBatchRegion = Platform::IntRectRegion();
     m_regularRenderJobsNotRenderedRegion = Platform::IntRectRegion();
-    m_parent->stopRenderTimer();
     ASSERT(isEmpty());
 }
 
@@ -375,7 +374,7 @@
         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 @@
     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 @@
 
     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::clearVisibleZoom()
 {
     m_visibleZoomJobs.clear();
-    if (isEmpty())
-         m_parent->stopRenderTimer();
 }
 
 bool RenderQueue::regularRenderJobsPreviouslyAttemptedButNotRendered(const Platform::IntRect& rect)
@@ -590,9 +584,6 @@
             renderRegularRenderJob();
     } else if (!m_nonVisibleScrollJobs.empty())
         renderNonVisibleScrollJob();
-
-    if (isEmpty())
-        m_parent->stopRenderTimer();
 }
 
 void RenderQueue::renderAllCurrentRegularRenderJobs()
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to