Changeset 293317 in webkit

Timestamp:

Apr 25, 2022, 12:04:37 AM (4 years ago)

Author:

commit-queue@webkit.org

Message:

REGRESSION (Safari 15.4) Performance regression after uploading WebGL buffers
​https://bugs.webkit.org/show_bug.cgi?id=239015

Patch by Kimmo Kinnunen <​kkinnunen@apple.com> on 2022-04-25
Reviewed by Dean Jackson.

BufferMtl::getRestartIndices() would recalculate restart indices on each
draw because RestartRangeCache::operator bool() would return m_isDirty
instead of the intended !m_isDirty.

Use std::optional to hold the RestartRangeCache instead of
maintaining m_isDirty. This allows the type system to contribute to the
correctness of the code. Dirty restart range cache is not useful and takes
up memory.

Re-apply hunks from r281550 "ANGLE Metal index buffer left mapped when building primitive restart ranges".

These were overwritten in r286638 "Roll ANGLE to include upstreamed Metal backend".

• src/libANGLE/renderer/metal/BufferMtl.h:

(rx::BufferMtl::RestartRangeCache::RestartRangeCache):

• src/libANGLE/renderer/metal/BufferMtl.mm:

(rx::BufferMtl::markConversionBuffersDirty):
(rx::BufferMtl::clearConversionBuffers):
(rx::BufferMtl::getRestartIndices):

Location:

trunk/Source/ThirdParty/ANGLE

Files:

• ChangeLog (modified) (1 diff)
• src/libANGLE/renderer/metal/BufferMtl.h (modified) (2 diffs)
• src/libANGLE/renderer/metal/BufferMtl.mm (modified) (4 diffs)

trunk/Source/ThirdParty/ANGLE/ChangeLog

@@ +1 @@
+2022-04-25  Kimmo Kinnunen  <kkinnunen@apple.com>
+
+        REGRESSION (Safari 15.4) Performance regression after uploading WebGL buffers
+        https://bugs.webkit.org/show_bug.cgi?id=239015
+
+        Reviewed by Dean Jackson.
+
+        BufferMtl::getRestartIndices() would recalculate restart indices on each
+        draw because RestartRangeCache::operator bool() would return m_isDirty
+        instead of the intended !m_isDirty.
+
+        Use std::optional to hold the RestartRangeCache instead of
+        maintaining m_isDirty. This allows the type system to contribute to the
+        correctness of the code. Dirty restart range cache is not useful and takes
+        up memory.
+
+        Re-apply hunks from r281550 "ANGLE Metal index buffer left mapped when building primitive restart ranges".
+
+        These were overwritten in r286638 "Roll ANGLE to include upstreamed Metal backend".
+
+        * src/libANGLE/renderer/metal/BufferMtl.h:
+        (rx::BufferMtl::RestartRangeCache::RestartRangeCache):
+        * src/libANGLE/renderer/metal/BufferMtl.mm:
+        (rx::BufferMtl::markConversionBuffersDirty):
+        (rx::BufferMtl::clearConversionBuffers):
+        (rx::BufferMtl::getRestartIndices):
+
 2022-04-18  Elliott Williams  <emw@apple.com>
 

trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/BufferMtl.h

@@ -13 +13 @@
 #import <Metal/Metal.h>
 
+#include <optional>
 #include <utility>
 
@@ -216 +217 @@
     struct RestartRangeCache
     {
-        RestartRangeCache() : indexType(gl::DrawElementsType::InvalidEnum) { isDirty = true; }
         RestartRangeCache(std::vector<IndexRange> &&ranges_, gl::DrawElementsType indexType_)
-            : ranges(ranges_), indexType(indexType_), isDirty(false)
+            : ranges(ranges_), indexType(indexType_)
         {}
-        void markDirty() { isDirty = true; }
-        operator bool() const { return isDirty; }
-        std::vector<IndexRange> ranges;
-        gl::DrawElementsType indexType;
-        bool isDirty;
+        const std::vector<IndexRange> ranges;
+        const gl::DrawElementsType indexType;
     };
-    RestartRangeCache mRestartRangeCache;
+    std::optional<RestartRangeCache> mRestartRangeCache;
     std::vector<IndexRange> mRestartIndices;
 };

trunk/Source/ThirdParty/ANGLE/src/libANGLE/renderer/metal/BufferMtl.mm

@@ -396 +396 @@
         buffer.convertedOffset = 0;
     }
-    mRestartRangeCache.markDirty();
+    mRestartRangeCache.reset();
 }
 
@@ -404 +404 @@
     mIndexConversionBuffers.clear();
     mUniformConversionBuffers.clear();
-    mRestartRangeCache.markDirty();
+    mRestartRangeCache.reset();
 }
 
@@ -434 +434 @@
                                                             gl::DrawElementsType indexType)
 {
-    if (!mRestartRangeCache || mRestartRangeCache.indexType != indexType)
-    {
-        mRestartRangeCache.markDirty();
+    if (!mRestartRangeCache || mRestartRangeCache->indexType != indexType)
+    {
+        mRestartRangeCache.reset();
         std::vector<IndexRange> ranges;
         switch (indexType)
@@ -452 +452 @@
                 ASSERT(false);
         }
-        mRestartRangeCache = RestartRangeCache(std::move(ranges), indexType);
-    }
-    return mRestartRangeCache.ranges;
+        mRestartRangeCache.emplace(std::move(ranges), indexType);
+    }
+    return mRestartRangeCache->ranges;
 }