Add perf warnings for getBufferSubData pipeline stalls

RESOLVED FIXED in Firefox 59

Status

()

enhancement
P3
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: jgilbert, Assigned: jgilbert)

Tracking

unspecified
mozilla59
Points:
---

Firefox Tracking Flags

(firefox59 fixed)

Details

(Whiteboard: gfx-noted)

Attachments

(2 attachments)

There is a good way (and many bad ways) to use getBufferSubData.
We should warn against the bay ways.
Specifically, the good way is using a buffer with a usage of GL_*_READ.

//////////////////////////////////////////////////

GLuint read_buffer;
glGenBuffers(1, &read_buffer);
glBindBuffer(GL_COPY_WRITE_BUFFER, read_buffer);
glBufferData(GL_COPY_WRITE_BUFFER, READ_SIZE, nullptr, GL_STREAM_READ);

// ...

if (pbos) {
  glBindBuffer(GL_PIXEL_PACK_BUFFER, read_buffer);
  glReadPixels(0, 0, W, H, GL_RGBA, GL_UNSIGNED_BYTE, 0);
} else {
  glBindBuffer(GL_COPY_WRITE_BUFFER, read_buffer);
  glCopyBufferSubData(source_target, GL_COPY_WRITE_BUFFER, source_offset, 0, N)
}
const auto fence = glFenceSync(GL_SYNC_GPU_COMMANDS_COMPLETE, 0);

// ...

GLenum fence_status;
glGetSynciv(fence, GL_SYNC_STATUS, 1, nullptr, (GLint*)&fence_status)
if (fence_status == GL_SIGNALED) {
  glBindBuffer(GL_COPY_WRITE_BUFFER, read_buffer);
  const auto data = glMapBufferRange(GL_COPY_WRITE_BUFFER, 0, READ_SIZE, GL_READ_BIT);
  // ...
  glUnmapBuffer(GL_COPY_WRITE_BUFFER)
}

//////////////////////////////////////////////////
Comment on attachment 8937091 [details]
Bug 1425488 - Warn when ill-advised readbacks will cause pipeline stalls. -

https://reviewboard.mozilla.org/r/207796/#review213840

lgtm! thanks
Attachment #8937091 - Flags: review?(dmu) → review+
Pushed by jgilbert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/12214b5efbe4
Warn when ill-advised readbacks will cause pipeline stalls. - r=daoshengmu
https://hg.mozilla.org/mozilla-central/rev/12214b5efbe4
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
Oops, this didn't actually ever mark buffers as written to.
Let's reopen and just fix it here.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 8938206 [details] [diff] [review]
0001-Bug-1425488-Reset-last-fenceId-on-write-to-buffers.-.patch

Review of attachment 8938206 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM, thanks.
Attachment #8938206 - Flags: review?(cleu) → review+
Pushed by jgilbert@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/51feeb32d06d
Reset last fenceId on write to buffers. - r=lenzak
https://hg.mozilla.org/mozilla-central/rev/51feeb32d06d
Status: REOPENED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.