Open Bug 1253463 Opened 9 years ago Updated 2 years ago

WebGL glBufferData() et al. which change size of a buffer fall off a performance cliff. (even when used with GL_STREAM_DRAW)

Categories

(Core :: Graphics: CanvasWebGL, defect, P3)

x86_64
macOS
defect

Tracking

()

Tracking Status
firefox47 --- affected

People

(Reporter: jujjyl, Unassigned)

Details

(Whiteboard: [gfx-noted])

Somewhat common GL usage is to create temporary VBO buffers with GL_STREAM_DRAW, in the form glBufferData(GL_ARRAY_BUFFER, size1, ptr1, GL_STREAM_DRAW); glDrawArrays(...); glBufferData(GL_ARRAY_BUFFER, size2, ptr2, GL_STREAM_DRAW); glDrawArrays(...); The idea being to upload one off data to GPU and hint the GPU with the GL_STREAM_DRAW that these buffers are to be used only once. Benchmarking a codebase from a partner, their code is dying on performance with this kind of access compared to native OpenGL. Tracing the code, it looks like this is due to the "bool sizeChanges" check in https://github.com/mozilla/gecko-dev/blob/master/dom/canvas/WebGLContextBuffers.cpp#L639 where glGetError() is called after the buffer creation. This is giving a large performance penalty when the GL command stream is flushed and presumably the CPU waits up for the GPU in lockstep for the error. Traced this feature back to https://bugzilla.mozilla.org/show_bug.cgi?id=665070, and I can appreciate the security requirement, but wanted to ask whether there is more recent understanding and something that we could do more clever here? The bug is from a long time ago in 2011. We have worked around this in the partner code by pooling VBOs manually and glBufferSubdata()ing the content in to avoid having to ever call glBufferData() with a changing size. This was benchmarked to increase GL performance in the application by +220%. Given that we have a workaround, this is not critical for the partner application, but after noticing how large of an impact this is, figured it would be worth revisiting about if we can be more clever here? Btw, are there other known cases where our WebGL implementation needs to call to glGetError() for such security reasons, which might cause stalls?
Jukka, do you have a test case that shows this? Also, have you tried removing that conditional (always making it false) and measure what kind of improvement we get? I understand the 220% improvement when the source code is modified, but I didn't know if you measured when our source is changed.
Flags: needinfo?(jujjyl)
Whiteboard: [gfx-noted]
This was done on a partner's closed source application as part of a work week visiting there (the recent thread you're on as well), and they did not let us have data out their office, so I don't have a test case. I did not measure against rebuilding Firefox without those checks, the way I pinpointed to that code was with gecko profiler, and the focus was on optimizing the partner's codebase which migrated to pooling the VBOs manually to work around this.
Flags: needinfo?(jujjyl)
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.