Title: [168657] trunk/Source/WebKit2
Revision
168657
Author
timothy_hor...@apple.com
Date
2014-05-12 16:31:56 -0700 (Mon, 12 May 2014)

Log Message

Triple-buffer RemoteLayerBackingStore
https://bugs.webkit.org/show_bug.cgi?id=132786
<rdar://problem/16877498>

Reviewed by Simon Fraser.

We need three buffers because we're currently unable to synchronize
with the render server to swap when they're not in use, so we were
throwing surfaces away far too frequently.

This hugely reduces time spent in IOSurface::create on various repaint benchmarks.

* Shared/mac/RemoteLayerBackingStore.h:
(WebKit::RemoteLayerBackingStore::hasFrontBuffer):
(WebKit::RemoteLayerBackingStore::volatility): Deleted.
* Shared/mac/RemoteLayerBackingStore.mm:
(WebKit::RemoteLayerBackingStore::RemoteLayerBackingStore):
(WebKit::RemoteLayerBackingStore::clearBackingStore):
(WebKit::RemoteLayerBackingStore::encode):
(WebKit::RemoteLayerBackingStore::decode):
(WebKit::RemoteLayerBackingStore::swapToValidFrontBuffer):
(WebKit::RemoteLayerBackingStore::display):
(WebKit::RemoteLayerBackingStore::applyBackingStoreToLayer):
(WebKit::RemoteLayerBackingStore::setBufferVolatility):
(WebKit::RemoteLayerBackingStore::Buffer::discard):
(WebKit::RemoteLayerBackingStore::setVolatility): Deleted.
* Shared/mac/RemoteLayerBackingStoreCollection.mm:
(WebKit::RemoteLayerBackingStoreCollection::purgeabilityTimerFired):
Put the ShareableBitmap/IOSurface and a volatility bit in a Buffer struct.
Also factor out "throw away this buffer and put it in the pool" into Buffer::discard().
We keep a volatility bit because querying IOSurface purgeability is expensive,
and we have a guarantee that any changes will happen in the Web process
and go through this class (the lack of this guarantee in most cases is why I'm not
putting this bit in WebCore::IOSurface itself).

Make it so that each buffer's volatility can be adjusted individually
by setBufferVolatility(), and adopt in RemoteLayerBackingStoreCollection.

Add a third buffer, m_secondaryBackBuffer, which will swap with the back buffer
before swapping front and back if the back buffer (soon to be the front buffer)
is still in use by the render server. This means that we will almost never
have to throw away a surface because it's in use (and conversely never need
to make a new surface).

Adjust RemoteLayerBackingStoreCollection to make secondary back surfaces purgeable
more aggressively than others.

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (168656 => 168657)


--- trunk/Source/WebKit2/ChangeLog	2014-05-12 23:29:18 UTC (rev 168656)
+++ trunk/Source/WebKit2/ChangeLog	2014-05-12 23:31:56 UTC (rev 168657)
@@ -1,3 +1,52 @@
+2014-05-12  Tim Horton  <timothy_hor...@apple.com>
+
+        Triple-buffer RemoteLayerBackingStore
+        https://bugs.webkit.org/show_bug.cgi?id=132786
+        <rdar://problem/16877498>
+
+        Reviewed by Simon Fraser.
+
+        We need three buffers because we're currently unable to synchronize
+        with the render server to swap when they're not in use, so we were
+        throwing surfaces away far too frequently.
+
+        This hugely reduces time spent in IOSurface::create on various repaint benchmarks.
+
+        * Shared/mac/RemoteLayerBackingStore.h:
+        (WebKit::RemoteLayerBackingStore::hasFrontBuffer):
+        (WebKit::RemoteLayerBackingStore::volatility): Deleted.
+        * Shared/mac/RemoteLayerBackingStore.mm:
+        (WebKit::RemoteLayerBackingStore::RemoteLayerBackingStore):
+        (WebKit::RemoteLayerBackingStore::clearBackingStore):
+        (WebKit::RemoteLayerBackingStore::encode):
+        (WebKit::RemoteLayerBackingStore::decode):
+        (WebKit::RemoteLayerBackingStore::swapToValidFrontBuffer):
+        (WebKit::RemoteLayerBackingStore::display):
+        (WebKit::RemoteLayerBackingStore::applyBackingStoreToLayer):
+        (WebKit::RemoteLayerBackingStore::setBufferVolatility):
+        (WebKit::RemoteLayerBackingStore::Buffer::discard):
+        (WebKit::RemoteLayerBackingStore::setVolatility): Deleted.
+        * Shared/mac/RemoteLayerBackingStoreCollection.mm:
+        (WebKit::RemoteLayerBackingStoreCollection::purgeabilityTimerFired):
+        Put the ShareableBitmap/IOSurface and a volatility bit in a Buffer struct.
+        Also factor out "throw away this buffer and put it in the pool" into Buffer::discard().
+        We keep a volatility bit because querying IOSurface purgeability is expensive,
+        and we have a guarantee that any changes will happen in the Web process
+        and go through this class (the lack of this guarantee in most cases is why I'm not
+        putting this bit in WebCore::IOSurface itself).
+
+        Make it so that each buffer's volatility can be adjusted individually
+        by setBufferVolatility(), and adopt in RemoteLayerBackingStoreCollection.
+
+        Add a third buffer, m_secondaryBackBuffer, which will swap with the back buffer
+        before swapping front and back if the back buffer (soon to be the front buffer)
+        is still in use by the render server. This means that we will almost never
+        have to throw away a surface because it's in use (and conversely never need
+        to make a new surface).
+
+        Adjust RemoteLayerBackingStoreCollection to make secondary back surfaces purgeable
+        more aggressively than others.
+
 2014-05-12  Alexey Proskuryakov  <a...@apple.com>
 
         REGRESSION (r165972): Can't type into text fields in Flash

Modified: trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.h (168656 => 168657)


--- trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.h	2014-05-12 23:29:18 UTC (rev 168656)
+++ trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.h	2014-05-12 23:31:56 UTC (rev 168657)
@@ -76,27 +76,27 @@
     {
 #if USE(IOSURFACE)
         if (m_acceleratesDrawing)
-            return !!m_frontSurface;
+            return !!m_frontBuffer.surface;
 #endif
-        return !!m_frontBuffer;
+        return !!m_frontBuffer.bitmap;
     }
 
     RetainPtr<CGContextRef> takeFrontContextPendingFlush();
 
-    enum class Volatility {
-        NonVolatile,
-        BackBufferVolatile,
-        AllBuffersVolatile
+    enum class BufferType {
+        Front,
+        Back,
+        SecondaryBack
     };
 
-    Volatility volatility() const { return m_volatility; }
-    bool setVolatility(Volatility);
+    bool setBufferVolatility(BufferType type, bool isVolatile);
 
     std::chrono::steady_clock::time_point lastDisplayTime() const { return m_lastDisplayTime; }
 
 private:
     void drawInContext(WebCore::GraphicsContext&, CGImageRef backImage);
     void clearBackingStore();
+    void swapToValidFrontBuffer();
 
     PlatformCALayerRemote* m_layer;
 
@@ -106,19 +106,26 @@
 
     WebCore::Region m_dirtyRegion;
 
-    RefPtr<ShareableBitmap> m_frontBuffer;
-    RefPtr<ShareableBitmap> m_backBuffer;
+    struct Buffer {
+        RefPtr<ShareableBitmap> bitmap;
 #if USE(IOSURFACE)
-    RefPtr<WebCore::IOSurface> m_frontSurface;
-    RefPtr<WebCore::IOSurface> m_backSurface;
+        RefPtr<WebCore::IOSurface> surface;
+        bool isVolatile = false;
 #endif
 
+        void discard();
+    };
+
+    Buffer m_frontBuffer;
+    Buffer m_backBuffer;
+#if USE(IOSURFACE)
+    Buffer m_secondaryBackBuffer;
+#endif
+
     RetainPtr<CGContextRef> m_frontContextPendingFlush;
 
     bool m_acceleratesDrawing;
 
-    Volatility m_volatility;
-
     WebCore::RepaintRectList m_paintingRects;
 
     RemoteLayerTreeContext* m_context;

Modified: trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.mm (168656 => 168657)


--- trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.mm	2014-05-12 23:29:18 UTC (rev 168656)
+++ trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStore.mm	2014-05-12 23:31:56 UTC (rev 168657)
@@ -58,7 +58,6 @@
 RemoteLayerBackingStore::RemoteLayerBackingStore(RemoteLayerTreeContext* context)
     : m_layer(nullptr)
     , m_isOpaque(false)
-    , m_volatility(RemoteLayerBackingStore::Volatility::NonVolatile)
     , m_context(context)
     , m_lastDisplayTime(std::chrono::steady_clock::time_point::min())
 {
@@ -90,17 +89,11 @@
 
 void RemoteLayerBackingStore::clearBackingStore()
 {
+    m_frontBuffer.discard();
+    m_backBuffer.discard();
 #if USE(IOSURFACE)
-    if (m_frontSurface)
-        IOSurfacePool::sharedPool().addSurface(m_frontSurface.get());
-    if (m_backSurface)
-        IOSurfacePool::sharedPool().addSurface(m_backSurface.get());
-
-    m_frontSurface = nullptr;
-    m_backSurface = nullptr;
+    m_secondaryBackBuffer.discard();
 #endif
-    m_frontBuffer = nullptr;
-    m_backBuffer = nullptr;
 }
 
 void RemoteLayerBackingStore::encode(IPC::ArgumentEncoder& encoder) const
@@ -112,7 +105,7 @@
 
 #if USE(IOSURFACE)
     if (m_acceleratesDrawing) {
-        mach_port_t port = m_frontSurface->createMachPort();
+        mach_port_t port = m_frontBuffer.surface->createMachPort();
         encoder << IPC::MachPort(port, MACH_MSG_TYPE_MOVE_SEND);
         return;
     }
@@ -121,7 +114,7 @@
     ASSERT(!m_acceleratesDrawing);
 
     ShareableBitmap::Handle handle;
-    m_frontBuffer->createHandle(handle);
+    m_frontBuffer.bitmap->createHandle(handle);
     encoder << handle;
 }
 
@@ -144,7 +137,7 @@
         IPC::MachPort machPort;
         if (!decoder.decode(machPort))
             return false;
-        result.m_frontSurface = IOSurface::createFromMachPort(machPort.port(), ColorSpaceDeviceRGB);
+        result.m_frontBuffer.surface = IOSurface::createFromMachPort(machPort.port(), ColorSpaceDeviceRGB);
         mach_port_deallocate(mach_task_self(), machPort.port());
         return true;
     }
@@ -155,7 +148,7 @@
     ShareableBitmap::Handle handle;
     if (!decoder.decode(handle))
         return false;
-    result.m_frontBuffer = ShareableBitmap::create(handle);
+    result.m_frontBuffer.bitmap = ShareableBitmap::create(handle);
 
     return true;
 }
@@ -170,13 +163,46 @@
     setNeedsDisplay(IntRect(IntPoint(), expandedIntSize(m_size)));
 }
 
+void RemoteLayerBackingStore::swapToValidFrontBuffer()
+{
+    FloatSize scaledSize = m_size;
+    scaledSize.scale(m_scale);
+    IntSize expandedScaledSize = roundedIntSize(scaledSize);
+
+#if USE(IOSURFACE)
+    if (m_acceleratesDrawing) {
+        if (!m_backBuffer.surface || m_backBuffer.surface->isInUse()) {
+            std::swap(m_backBuffer, m_secondaryBackBuffer);
+            if (m_backBuffer.surface && m_backBuffer.surface->isInUse())
+                m_backBuffer.discard();
+        }
+
+        std::swap(m_frontBuffer, m_backBuffer);
+
+        if (!m_frontBuffer.surface)
+            m_frontBuffer.surface = IOSurface::create(expandedScaledSize, ColorSpaceDeviceRGB);
+
+        setBufferVolatility(BufferType::Front, false);
+
+        return;
+    }
+#endif
+
+    ASSERT(!m_acceleratesDrawing);
+    std::swap(m_frontBuffer, m_backBuffer);
+
+    if (!m_frontBuffer.bitmap)
+        m_frontBuffer.bitmap = ShareableBitmap::createShareable(expandedScaledSize, m_isOpaque ? ShareableBitmap::NoFlags : ShareableBitmap::SupportsAlpha);
+}
+
 bool RemoteLayerBackingStore::display()
 {
     ASSERT(!m_frontContextPendingFlush);
 
     m_lastDisplayTime = std::chrono::steady_clock::now();
 
-    setVolatility(Volatility::NonVolatile);
+    // Make the previous front buffer non-volatile early, so that we can dirty the whole layer if it comes back empty.
+    setBufferVolatility(BufferType::Front, false);
 
     if (m_dirtyRegion.isEmpty() || m_size.isEmpty())
         return false;
@@ -195,42 +221,33 @@
     IntSize expandedScaledSize = roundedIntSize(scaledSize);
     IntRect expandedScaledLayerBounds(IntPoint(), expandedScaledSize);
     bool willPaintEntireBackingStore = m_dirtyRegion.contains(layerBounds);
+
+    swapToValidFrontBuffer();
+
 #if USE(IOSURFACE)
     if (m_acceleratesDrawing) {
-        std::swap(m_frontSurface, m_backSurface);
-
-        if (!m_frontSurface || m_frontSurface->isInUse()) {
-            if (m_frontSurface)
-                IOSurfacePool::sharedPool().addSurface(m_frontSurface.get());
-            m_frontSurface = IOSurface::create(expandedScaledSize, ColorSpaceDeviceRGB);
-        }
-
         RetainPtr<CGImageRef> backImage;
-        if (m_backSurface && !willPaintEntireBackingStore)
-            backImage = m_backSurface->createImage();
+        if (m_backBuffer.surface && !willPaintEntireBackingStore)
+            backImage = m_backBuffer.surface->createImage();
 
-        GraphicsContext& context = m_frontSurface->ensureGraphicsContext();
+        GraphicsContext& context = m_frontBuffer.surface->ensureGraphicsContext();
 
         context.scale(FloatSize(1, -1));
         context.translate(0, -expandedScaledSize.height());
         drawInContext(context, backImage.get());
 
-        m_frontSurface->clearGraphicsContext();
+        m_frontBuffer.surface->clearGraphicsContext();
 
         return true;
     }
 #endif
 
     ASSERT(!m_acceleratesDrawing);
+    std::unique_ptr<GraphicsContext> context = m_frontBuffer.bitmap->createGraphicsContext();
 
-    std::swap(m_frontBuffer, m_backBuffer);
-    if (!m_frontBuffer)
-        m_frontBuffer = ShareableBitmap::createShareable(expandedScaledSize, m_isOpaque ? ShareableBitmap::NoFlags : ShareableBitmap::SupportsAlpha);
-    std::unique_ptr<GraphicsContext> context = m_frontBuffer->createGraphicsContext();
-
     RetainPtr<CGImageRef> backImage;
-    if (m_backBuffer && !willPaintEntireBackingStore)
-        backImage = m_backBuffer->makeCGImage();
+    if (m_backBuffer.bitmap && !willPaintEntireBackingStore)
+        backImage = m_backBuffer.bitmap->makeCGImage();
 
     drawInContext(*context, backImage.get());
     
@@ -345,13 +362,13 @@
 
 #if USE(IOSURFACE)
     if (acceleratesDrawing()) {
-        layer.contents = (id)m_frontSurface->surface();
+        layer.contents = (id)m_frontBuffer.surface->surface();
         return;
     }
 #endif
 
     ASSERT(!acceleratesDrawing());
-    layer.contents = (id)m_frontBuffer->makeCGImageCopy().get();
+    layer.contents = (id)m_frontBuffer.bitmap->makeCGImageCopy().get();
 }
 
 RetainPtr<CGContextRef> RemoteLayerBackingStore::takeFrontContextPendingFlush()
@@ -360,40 +377,59 @@
 }
 
 #if USE(IOSURFACE)
-bool RemoteLayerBackingStore::setVolatility(Volatility volatility)
+bool RemoteLayerBackingStore::setBufferVolatility(BufferType type, bool isVolatile)
 {
-    if (m_volatility == volatility)
-        return true;
+    switch(type) {
+    case BufferType::Front:
+        if (m_frontBuffer.surface && m_frontBuffer.isVolatile != isVolatile) {
+            if (!isVolatile || !m_frontBuffer.surface->isInUse()) {
+                IOSurface::SurfaceState previousState = m_frontBuffer.surface->setIsVolatile(isVolatile);
+                m_frontBuffer.isVolatile = isVolatile;
 
-    bool wantsVolatileFrontBuffer = volatility == Volatility::AllBuffersVolatile;
-    bool wantsVolatileBackBuffer = volatility == Volatility::AllBuffersVolatile || volatility == Volatility::BackBufferVolatile;
-
-    // If either surface is in-use and would become volatile, don't make any changes.
-    if (wantsVolatileFrontBuffer && m_frontSurface && m_frontSurface->isInUse())
-        return false;
-    if (wantsVolatileBackBuffer && m_backSurface && m_backSurface->isInUse())
-        return false;
-
-    if (m_frontSurface) {
-        IOSurface::SurfaceState previousState = m_frontSurface->setIsVolatile(wantsVolatileFrontBuffer);
-
-        // Becoming non-volatile and the front buffer was purged, so we need to repaint.
-        if (!wantsVolatileFrontBuffer && (previousState == IOSurface::SurfaceState::Empty))
-            setNeedsDisplay();
+                // Becoming non-volatile and the front buffer was purged, so we need to repaint.
+                if (!isVolatile && (previousState == IOSurface::SurfaceState::Empty))
+                    setNeedsDisplay();
+            } else
+                return false;
+        }
+        break;
+    case BufferType::Back:
+        if (m_backBuffer.surface && m_backBuffer.isVolatile != isVolatile) {
+            if (!isVolatile || !m_backBuffer.surface->isInUse()) {
+                m_backBuffer.surface->setIsVolatile(isVolatile);
+                m_backBuffer.isVolatile = isVolatile;
+            } else
+                return false;
+        }
+        break;
+    case BufferType::SecondaryBack:
+        if (m_secondaryBackBuffer.surface && m_secondaryBackBuffer.isVolatile != isVolatile) {
+            if (!isVolatile || !m_secondaryBackBuffer.surface->isInUse()) {
+                m_secondaryBackBuffer.surface->setIsVolatile(isVolatile);
+                m_secondaryBackBuffer.isVolatile = isVolatile;
+            } else
+                return false;
+        }
+        break;
     }
-
-    if (m_backSurface)
-        m_backSurface->setIsVolatile(wantsVolatileBackBuffer);
-
-    m_volatility = volatility;
-
     return true;
 }
 #else
-bool RemoteLayerBackingStore::setVolatility(Volatility)
+bool RemoteLayerBackingStore::setBufferVolatility(BufferType, bool)
 {
     return true;
 }
 #endif
 
+void RemoteLayerBackingStore::Buffer::discard()
+{
+#if USE(IOSURFACE)
+    if (surface)
+        IOSurfacePool::sharedPool().addSurface(surface.get());
+    surface = nullptr;
+    isVolatile = false;
+#endif
+    bitmap = nullptr;
+}
+
 } // namespace WebKit

Modified: trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStoreCollection.mm (168656 => 168657)


--- trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStoreCollection.mm	2014-05-12 23:29:18 UTC (rev 168656)
+++ trunk/Source/WebKit2/Shared/mac/RemoteLayerBackingStoreCollection.mm	2014-05-12 23:31:56 UTC (rev 168657)
@@ -31,7 +31,8 @@
 #import "RemoteLayerTreeContext.h"
 
 const std::chrono::seconds purgeableBackingStoreAgeThreshold = 1_s;
-const std::chrono::seconds purgeabilityTimerInterval = 1_s;
+const std::chrono::milliseconds purgeableSecondaryBackingStoreAgeThreshold = 200_ms;
+const std::chrono::milliseconds purgeabilityTimerInterval = 200_ms;
 
 namespace WebKit {
 
@@ -60,12 +61,18 @@
     for (const auto& backingStore : m_liveBackingStore) {
         if (now - backingStore->lastDisplayTime() < purgeableBackingStoreAgeThreshold) {
             hadRecentlyPaintedBackingStore = true;
+
+            if (now - backingStore->lastDisplayTime() >= purgeableSecondaryBackingStoreAgeThreshold)
+                backingStore->setBufferVolatility(RemoteLayerBackingStore::BufferType::SecondaryBack, true);
+
             continue;
         }
 
         // FIXME: If the layer is unparented, we should make all buffers volatile.
-        if (!backingStore->setVolatility(RemoteLayerBackingStore::Volatility::BackBufferVolatile))
+        if (!backingStore->setBufferVolatility(RemoteLayerBackingStore::BufferType::SecondaryBack, true))
             successfullyMadeBackingStorePurgeable = false;
+        if (!backingStore->setBufferVolatility(RemoteLayerBackingStore::BufferType::Back, true))
+            successfullyMadeBackingStorePurgeable = false;
     }
 
     if (!hadRecentlyPaintedBackingStore && successfullyMadeBackingStorePurgeable)
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to