gpu: Thread-safe command buffer state lookup.

gpu/ipc/in_process_command_buffer.cc

Index: gpu/ipc/in_process_command_buffer.cc
diff --git a/gpu/ipc/in_process_command_buffer.cc b/gpu/ipc/in_process_command_buffer.cc
index a30b1e17e7ba1a1a4d304a13f5f999ea5b8ebefd..5567d717ceef40ad6441c255d1e30de7b8ecf8dc 100644
--- a/gpu/ipc/in_process_command_buffer.cc
+++ b/gpu/ipc/in_process_command_buffer.cc
@@ -543,13 +543,8 @@ void InProcessCommandBuffer::ProcessTasksOnGpuThread() {
 
 CommandBuffer::State InProcessCommandBuffer::GetLastState() {
   CheckSequencedThread();
-  return last_state_;
-}
-
-int32_t InProcessCommandBuffer::GetLastToken() {
-  CheckSequencedThread();
   GetStateFast();
-  return last_state_.token;
+  return last_state_;

[piman, 2016/12/07 20:30:13]

nit: this whole function is fully equivalent to GetStateFast() - maybe combine?

 }
 
 void InProcessCommandBuffer::FlushOnGpuThread(int32_t put_offset) {
@@ -617,24 +612,25 @@ void InProcessCommandBuffer::OrderingBarrier(int32_t put_offset) {
   Flush(put_offset);
 }
 
-void InProcessCommandBuffer::WaitForTokenInRange(int32_t start, int32_t end) {
+CommandBuffer::State InProcessCommandBuffer::WaitForTokenInRange(int32_t start,
+                                                                 int32_t end) {
   CheckSequencedThread();
-  while (!InRange(start, end, GetLastToken()) &&
-         last_state_.error == gpu::error::kNoError) {
+  while (!InRange(start, end, GetLastState().token) &&
+         GetLastState().error == gpu::error::kNoError) {

[piman, 2016/12/07 20:30:13]

Should we avoid taking the lock twice (2 calls to GetLastState) per loop?
Something like:

State state = GetLastState();
while (!InRange(start, end, state.token) &&
       state.error == gpu::error::kNoError) {
  flush_event_.Wait();
  state = GetLastState();
}

[sunnyps, 2016/12/08 01:14:16]

Done.

     flush_event_.Wait();
   }
+  return last_state_;
 }
 
-void InProcessCommandBuffer::WaitForGetOffsetInRange(int32_t start,
-                                                     int32_t end) {
+CommandBuffer::State InProcessCommandBuffer::WaitForGetOffsetInRange(
+    int32_t start,
+    int32_t end) {
   CheckSequencedThread();
-
-  GetStateFast();
-  while (!InRange(start, end, last_state_.get_offset) &&
-         last_state_.error == gpu::error::kNoError) {
+  while (!InRange(start, end, GetLastState().get_offset) &&
+         GetLastState().error == gpu::error::kNoError) {

[piman, 2016/12/07 20:30:13]

Ditto

[sunnyps, 2016/12/08 01:14:16]

Done.

     flush_event_.Wait();
-    GetStateFast();
   }
+  return last_state_;
 }
 
 void InProcessCommandBuffer::SetGetBuffer(int32_t shm_id) {
@@ -1030,6 +1026,11 @@ bool InProcessCommandBuffer::IsFenceSyncFlushReceived(uint64_t release) {
   return IsFenceSyncFlushed(release);
 }
 
+bool InProcessCommandBuffer::IsFenceSyncReleased(uint64_t release) {

[piman, 2016/12/07 20:30:13]

We will need this one to be thread-safe too for mus things. I think it should be
fairly straightforward (a lock around last_state_, similar to
CommandBufferProxyImpl). Maybe it's ok to leave as a TODO?

I guess that answers my question of whether or not there are non-thread-safe
implementations, but it means we need to be honest about it all the way to
ContextSupport - and maybe provide a way to introspect, or at least DCHECK that
capability.

[sunnyps, 2016/12/08 01:14:16]

Made this thread-safe although there's no real benefit to it right now because
last_state_ is only updated after a flush. We can make last_state_ SharedState
if we need to.

+  State state = GetStateFast();
+  return release <= state.release_count;
+}
+
 void InProcessCommandBuffer::SignalSyncToken(const SyncToken& sync_token,
                                              const base::Closure& callback) {
   CheckSequencedThread();
@@ -1132,11 +1133,6 @@ void InProcessCommandBuffer::SetUpdateVSyncParametersCallback(
   update_vsync_parameters_completion_callback_ = callback;
 }
 
-gpu::error::Error InProcessCommandBuffer::GetLastError() {
-  CheckSequencedThread();
-  return last_state_.error;
-}
-
 namespace {
 
 void PostCallback(