Make CommandBuffer::Flush asynchronous, and add CommandBuffer::FlushSync with former semantics.

Make CommandBuffer::Flush asynchronous, and add CommandBuffer::FlushSync with former semantics.

Also force a flush when the buffer is getting full.

BUG=none
TEST=gpu_unittests, some webgl content, some 3d css content, pepper flash

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=71514

[piman, 2011-01-14 04:55:04]

I'm also working on a CL that would replace the uses of FlushSync (in all the
waiting calls) by actual waiting calls, but that will come later.
This CL gives ~5% improvement on some Flash content, and I suspect it also helps
accelerated web content as well.

[apatrick, 2011-01-14 18:31:34]

Awesome thanks for doing this. Just some notes and observations. LGTM.

http://codereview.chromium.org/6316002/diff/1/gpu/command_buffer/client/cmd_b...
File gpu/command_buffer/client/cmd_buffer_helper.cc (right):

http://codereview.chromium.org/6316002/diff/1/gpu/command_buffer/client/cmd_b...
gpu/command_buffer/client/cmd_buffer_helper.cc:116: GPU_CHECK(count <
usable_entry_count_);
I was thinking about putting an async flush in here once the command buffer is,
say, half full. If you're in the area :)

http://codereview.chromium.org/6316002/diff/1/gpu/command_buffer/client/cmd_b...
gpu/command_buffer/client/cmd_buffer_helper.cc:123: Flush();
Won't this async flush likely be immediately followed by the sync flush below?

http://codereview.chromium.org/6316002/diff/1/gpu/command_buffer/client/cmd_b...
gpu/command_buffer/client/cmd_buffer_helper.cc:137: Flush();
Same here. And some other places in this file.

http://codereview.chromium.org/6316002/diff/1/gpu/command_buffer/client/cmd_b...
File gpu/command_buffer/client/cmd_buffer_helper.h (right):

http://codereview.chromium.org/6316002/diff/1/gpu/command_buffer/client/cmd_b...
gpu/command_buffer/client/cmd_buffer_helper.h:45: // Flushes the commands,
setting the put pointer to let the buffer interface
This comment is out-of-date now that Flush is async.

http://codereview.chromium.org/6316002/diff/1/gpu/pgl/command_buffer_pepper.cc
File gpu/pgl/command_buffer_pepper.cc (right):

http://codereview.chromium.org/6316002/diff/1/gpu/pgl/command_buffer_pepper.c...
gpu/pgl/command_buffer_pepper.cc:86: void CommandBufferPepper::Flush(int32
put_offset) {
I think we might want to change PPAPI to make the distinction between sync and
async flushes.

[Antoine Labour, 2011-01-14 21:35:33]

http://codereview.chromium.org/6316002/diff/1/gpu/command_buffer/client/cmd_b...
File gpu/command_buffer/client/cmd_buffer_helper.cc (right):

http://codereview.chromium.org/6316002/diff/1/gpu/command_buffer/client/cmd_b...
gpu/command_buffer/client/cmd_buffer_helper.cc:116: GPU_CHECK(count <
usable_entry_count_);
On 2011/01/14 18:31:34, apatrick wrote:
> I was thinking about putting an async flush in here once the command buffer
is,
> say, half full. If you're in the area :)

Done. (added at the end of the function)

http://codereview.chromium.org/6316002/diff/1/gpu/command_buffer/client/cmd_b...
gpu/command_buffer/client/cmd_buffer_helper.cc:123: Flush();
On 2011/01/14 18:31:34, apatrick wrote:
> Won't this async flush likely be immediately followed by the sync flush below?

Done. You're right we don't need to do it, and it makes the code simpler.

http://codereview.chromium.org/6316002/diff/1/gpu/command_buffer/client/cmd_b...
gpu/command_buffer/client/cmd_buffer_helper.cc:137: Flush();
On 2011/01/14 18:31:34, apatrick wrote:
> Same here. And some other places in this file.

Done.

http://codereview.chromium.org/6316002/diff/1/gpu/command_buffer/client/cmd_b...
File gpu/command_buffer/client/cmd_buffer_helper.h (right):

http://codereview.chromium.org/6316002/diff/1/gpu/command_buffer/client/cmd_b...
gpu/command_buffer/client/cmd_buffer_helper.h:45: // Flushes the commands,
setting the put pointer to let the buffer interface
On 2011/01/14 18:31:34, apatrick wrote:
> This comment is out-of-date now that Flush is async.

Done.

http://codereview.chromium.org/6316002/diff/1/gpu/pgl/command_buffer_pepper.cc
File gpu/pgl/command_buffer_pepper.cc (right):

http://codereview.chromium.org/6316002/diff/1/gpu/pgl/command_buffer_pepper.c...
gpu/pgl/command_buffer_pepper.cc:86: void CommandBufferPepper::Flush(int32
put_offset) {
On 2011/01/14 18:31:34, apatrick wrote:
> I think we might want to change PPAPI to make the distinction between sync and
> async flushes.

This is old Pepper v1 code. It will go away in a few days, I'm just keeping it
compiling and working semantically.

For PPAPI (Pepper v2) code, the command buffers aren't exposed directly to the
client. All the client has is higher level glFlush, glFinish and other blocking
calls - and possibly fences if we add them. I don't think there's a point in
exposing the "sync" flush as such to the client, but it is currently necessary
for the underlying implementation to avoid busy-waiting (though I want to
replace it with Wait interfaces - e.g. wait-on-get-reaching-that-value or
wait-on-token-reaching-that-value - that would reduce IPC traffic and improve
latency a little bit).

http://codereview.chromium.org/6316002/diff/7001/gpu/command_buffer/client/cm...
File gpu/command_buffer/client/cmd_buffer_helper.cc (right):

http://codereview.chromium.org/6316002/diff/7001/gpu/command_buffer/client/cm...
gpu/command_buffer/client/cmd_buffer_helper.cc:145: if (pending > limit) {
Note: we only get here if we haven't synced above.

[apatrick_chromium, 2011-01-14 22:43:32]

LGTM

http://codereview.chromium.org/6316002/diff/11001/gpu/command_buffer/client/c...
File gpu/command_buffer/client/cmd_buffer_helper.cc (right):

http://codereview.chromium.org/6316002/diff/11001/gpu/command_buffer/client/c...
gpu/command_buffer/client/cmd_buffer_helper.cc:141: // reader is know to be
idle.
know -> known

[Antoine Labour, 2011-01-14 23:17:27]

PTAL (sorry). The unittests were failing because they relied on implementation
details. Adding the extra Flush made the unittest assumptions invalid.

http://codereview.chromium.org/6316002/diff/11001/gpu/command_buffer/client/c...
File gpu/command_buffer/client/cmd_buffer_helper.cc (right):

http://codereview.chromium.org/6316002/diff/11001/gpu/command_buffer/client/c...
gpu/command_buffer/client/cmd_buffer_helper.cc:141: // reader is know to be
idle.
On 2011/01/14 22:43:32, apatrick_chromium wrote:
> know -> known

Done.

[apatrick_chromium, 2011-01-14 23:29:31]

LGTM