Workaround for chrome on mac buffer object performance issue.

Review URL: http://codereview.appspot.com/5713051/

git-svn-id: http://skia.googlecode.com/svn/trunk@3289 2bbb7eff-a529-9590-31e7-b0007b416f81

diff --git a/include/gpu/gl/GrGLConfig.h b/include/gpu/gl/GrGLConfig.h
index 5ae931c607007598184b55289f4670f98c2b0b87..2cc4f31edac9de499fd00cc009736ade31e4168c 100644 (file)
--- a/include/gpu/gl/GrGLConfig.h
+++ b/include/gpu/gl/GrGLConfig.h
@@ -149,6 +149,39 @@
     #define GR_GL_CHECK_FBO_STATUS_ONCE_PER_FORMAT      0
 #endif
 
+/**
+ * There is a strange bug that occurs on Macs with NVIDIA GPUs. We don't
+ * fully understand it. When (element) array buffers are continually
+ * respecified using glBufferData at the same (or perhaps just a small) size
+ * performance can fall off of a cliff. The driver winds up performing many
+ * DMA mapping / unmappings and chews up ~50% of the core. However, it has been
+ * observed that respecifiying the buffer with a larger size and then writing
+ * the small amount of actual data using glBufferSubData prevents the bad
+ * behavior.
+ *
+ * There is a lot of uncertainty around this issue. In Chrome backgrounding
+ * the tab somehow initiates this behavior and we don't know what the connection
+ * is. Another observation is that Chrome's cmd buffer server will actually
+ * create a buffer full of zeros when it sees a NULL data param (for security
+ * reasons). If this is disabled and NULL is actually passed all the way to the
+ * driver then the workaround doesn't help.
+ *
+ * The issue is tracked at:
+ * http://code.google.com/p/chromium/issues/detail?id=114865
+ *
+ * When GR_GL_USE_BUFFER_DATA_NULL_HINT is not set we encounter the bug
+ * if we keep uploading small vetex / index sizes. So, every 1K uploads we
+ * act as though GR_GL_USE_BUFFER_DATA_NULL_HINT was set: we specify the buffer
+ * at full size with a NULL data ptr and then do a partial update with
+ * glBufferSubData.
+ *
+ * Hopefully we will understand this better and have a cleaner fix or get a
+ * OS/driver level fix.
+ */
+#define GR_GL_MAC_BUFFER_OBJECT_PERFOMANCE_WORKAROUND   \
+    (GR_MAC_BUILD &&                                    \
+     !GR_GL_USE_BUFFER_DATA_NULL_HINT)
+
 #if(GR_GL_NO_CONSTANT_ATTRIBUTES) && (GR_GL_ATTRIBUTE_MATRICES)
     #error "Cannot combine GR_GL_NO_CONSTANT_ATTRIBUTES and GR_GL_ATTRIBUTE_MATRICES"
 #endif

diff --git a/src/gpu/gl/GrGLIndexBuffer.cpp b/src/gpu/gl/GrGLIndexBuffer.cpp
index b64668ede2c18ec7540845c8468852152e9d19c5..f424e78cefc647ad0efe820b44b406d410a206d1 100644 (file)
--- a/src/gpu/gl/GrGLIndexBuffer.cpp
+++ b/src/gpu/gl/GrGLIndexBuffer.cpp
@@ -104,28 +104,38 @@ bool GrGLIndexBuffer::updateData(const void* src, size_t srcSizeInBytes) {
     }
     this->bind();
     GrGLenum usage = dynamic() ? GR_GL_DYNAMIC_DRAW : GR_GL_STATIC_DRAW;
-#if !GR_GL_USE_BUFFER_DATA_NULL_HINT
-    // Note that we're cheating on the size here. Currently no methods
-    // allow a partial update that preserves contents of non-updated
-    // portions of the buffer (and lock() does a glBufferData(..size, NULL..))
-    GL_CALL(BufferData(GR_GL_ELEMENT_ARRAY_BUFFER, srcSizeInBytes, src, usage));
-#else
-    if (this->sizeInBytes() == srcSizeInBytes) {
-        GL_CALL(BufferData(GR_GL_ELEMENT_ARRAY_BUFFER,
-                            srcSizeInBytes, src, usage));
+
+    bool doNullHint = GR_GL_USE_BUFFER_DATA_NULL_HINT;
+#if GR_GL_MAC_BUFFER_OBJECT_PERFOMANCE_WORKAROUND
+    GrAssert(!doNullHint);
+    static int N = 0;
+    doNullHint = (0 == N % 1024);
+    ++N;
+#endif
+
+    if (doNullHint) {
+        if (this->sizeInBytes() == srcSizeInBytes) {
+            GL_CALL(BufferData(GR_GL_ELEMENT_ARRAY_BUFFER,
+                                srcSizeInBytes, src, usage));
+        } else {
+            // Before we call glBufferSubData we give the driver a hint using
+            // glBufferData with NULL. This makes the old buffer contents
+            // inaccessible to future draws. The GPU may still be processing
+            // draws that reference the old contents. With this hint it can
+            // assign a different allocation for the new contents to avoid
+            // flushing the gpu past draws consuming the old contents.
+            GL_CALL(BufferData(GR_GL_ELEMENT_ARRAY_BUFFER,
+                               this->sizeInBytes(), NULL, usage));
+            GL_CALL(BufferSubData(GR_GL_ELEMENT_ARRAY_BUFFER,
+                                  0, srcSizeInBytes, src));
+        }
     } else {
-        // Before we call glBufferSubData we give the driver a hint using
-        // glBufferData with NULL. This makes the old buffer contents
-        // inaccessible to future draws. The GPU may still be processing draws
-        // that reference the old contents. With this hint it can assign a
-        // different allocation for the new contents to avoid flushing the gpu
-        // past draws consuming the old contents.
-        GL_CALL(BufferData(GR_GL_ELEMENT_ARRAY_BUFFER,
-                           this->sizeInBytes(), NULL, usage));
-        GL_CALL(BufferSubData(GR_GL_ELEMENT_ARRAY_BUFFER,
-                              0, srcSizeInBytes, src));
+        // Note that we're cheating on the size here. Currently no methods
+        // allow a partial update that preserves contents of non-updated
+        // portions of the buffer (lock() does a glBufferData(..size, NULL..))
+        GL_CALL(BufferData(GR_GL_ELEMENT_ARRAY_BUFFER, 
+                           srcSizeInBytes, src, usage));
     }
-#endif
     return true;
 }
 

diff --git a/src/gpu/gl/GrGLVertexBuffer.cpp b/src/gpu/gl/GrGLVertexBuffer.cpp
index 33c1e7e3e00c251b69d4f84c09d4d301e60c485f..492886853938db7c6b9be7c8896601d3bfd41d17 100644 (file)
--- a/src/gpu/gl/GrGLVertexBuffer.cpp
+++ b/src/gpu/gl/GrGLVertexBuffer.cpp
@@ -101,26 +101,34 @@ bool GrGLVertexBuffer::updateData(const void* src, size_t srcSizeInBytes) {
     }
     this->bind();
     GrGLenum usage = dynamic() ? GR_GL_DYNAMIC_DRAW : GR_GL_STATIC_DRAW;
-#if !GR_GL_USE_BUFFER_DATA_NULL_HINT
-    // Note that we're cheating on the size here. Currently no methods
-    // allow a partial update that preserves contents of non-updated
-    // portions of the buffer (and lock() does a glBufferData(..size, NULL..))
-    GL_CALL(BufferData(GR_GL_ARRAY_BUFFER, srcSizeInBytes, src, usage));
-#else
-    if (this->sizeInBytes() == srcSizeInBytes) {
-        GL_CALL(BufferData(GR_GL_ARRAY_BUFFER, srcSizeInBytes, src, usage));
+
+    bool doNullHint = GR_GL_USE_BUFFER_DATA_NULL_HINT;
+#if GR_GL_MAC_BUFFER_OBJECT_PERFOMANCE_WORKAROUND
+    GrAssert(!doNullHint);
+    static int N = 0;
+    doNullHint = (0 == N % 1024);
+    ++N;
+#endif
+    if (doNullHint) {
+        if (this->sizeInBytes() == srcSizeInBytes) {
+            GL_CALL(BufferData(GR_GL_ARRAY_BUFFER, srcSizeInBytes, src, usage));
+        } else {
+            // Before we call glBufferSubData we give the driver a hint using
+            // glBufferData with NULL. This makes the old buffer contents
+            // inaccessible to future draws. The GPU may still be processing
+            // draws that reference the old contents. With this hint it can
+            // assign a different allocation for the new contents to avoid
+            // flushing the gpu past draws consuming the old contents.
+            GL_CALL(BufferData(GR_GL_ARRAY_BUFFER, 
+                               this->sizeInBytes(), NULL, usage));
+            GL_CALL(BufferSubData(GR_GL_ARRAY_BUFFER, 0, srcSizeInBytes, src));
+        }
     } else {
-        // Before we call glBufferSubData we give the driver a hint using
-        // glBufferData with NULL. This makes the old buffer contents
-        // inaccessible to future draws. The GPU may still be processing draws
-        // that reference the old contents. With this hint it can assign a
-        // different allocation for the new contents to avoid flushing the gpu
-        // past draws consuming the old contents.
-        GL_CALL(BufferData(GR_GL_ARRAY_BUFFER, 
-                           this->sizeInBytes(), NULL, usage));
-        GL_CALL(BufferSubData(GR_GL_ARRAY_BUFFER, 0, srcSizeInBytes, src));
+        // Note that we're cheating on the size here. Currently no methods
+        // allow a partial update that preserves contents of non-updated
+        // portions of the buffer (lock() does a glBufferData(..size, NULL..))
+        GL_CALL(BufferData(GR_GL_ARRAY_BUFFER, srcSizeInBytes, src, usage));
     }
-#endif
     return true;
 }